Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: README.md

Issue 29716587: Noissue - bundle in one operation to ensure no races (Closed)
Patch Set: Created March 7, 2018, 2:46 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | package.json » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 Shared Adblock Plus UI code 1 Shared Adblock Plus UI code
2 =========================== 2 ===========================
3 3
4 The user interface elements defined in this repository will be used by various 4 The user interface elements defined in this repository will be used by various
5 Adblock Plus products like Adblock Plus for Firefox. Their functionality can be 5 Adblock Plus products like Adblock Plus for Firefox. Their functionality can be
6 tested within this repository, even though they might not work exactly the same 6 tested within this repository, even though they might not work exactly the same
7 as they will do in the final product. 7 as they will do in the final product.
8 8
9 Installing dependencies 9 Installing dependencies
10 ----------------------- 10 -----------------------
(...skipping 72 matching lines...) Expand 10 before | Expand all | Expand 10 after
83 npx stylelint --fix skin/real-file-name.css 83 npx stylelint --fix skin/real-file-name.css
84 ``` 84 ```
85 85
86 Bundling JS 86 Bundling JS
87 ----------- 87 -----------
88 88
89 As it is for the `desktop-options.js` case, bundling is done via `package.json` 89 As it is for the `desktop-options.js` case, bundling is done via `package.json`
90 script entries. 90 script entries.
91 91
92 A dedicated script entry, such `bundle:desktop-options.js`, 92 A dedicated script entry, such `bundle:desktop-options.js`,
93 should perform the following operations: 93 should simply use the `bash:js` script, passing along
94 the source file and the target.
94 95
95 * ensure source code passes linting 96 ```js
96 * ensure the bundle won't affect future linting operations 97 {
97 * ensure `browserify` uses `--node` and `--no-bundle-external` flags 98 // example of a new bundle for the ./js/source.js file
98 * point at the entry point, and output in the top level folder 99 "bundle:target.js": "npm run bash:js ./js/source.js ./target.js"
99 100 }
100 Accordingly, this is what happens with the `bundle:desktop-options.js`:
101
102 ```sh
103 # the && operator ensure each step is executed only
104 # if the previous one didn't produce an error
105 eslint ./js/**/*.js &&
106 # browserify doesn't offer a way to prefix with text
107 # the file is hence created with eslint disabled and a warning
108 echo '/* eslint-disable */// BUNDLED FILE' > ./desktop-options.js &&
109 # browserify take an entry point and bundle all its required files
110 # outputting the result into ./desktop-options.js
111 browserify --node --no-bundle-external js/desktop-options.js >> ./desktop-option s.js
112 ``` 101 ```
113 102
114 For a new bundle, i.e. `mobile-options.js`, simply use the same procedure 103 The main `bundle` script should include each sub-bundle operation too.
115 but swap the `desktop-options.js` file/script name with `mobile-options.js`.
116
117 The main `bundle` script should include each sub-bundle operation.
118 104
119 Bundling CSS 105 Bundling CSS
120 ------------ 106 ------------
121 107
122 As it is for the `desktop-options.css` case, bundling is done via `package.json` 108 As it is for the `desktop-options.css` case, bundling is done via `package.json`
123 script entries. 109 script entries.
124 110
125 A dedicated script entry, such `bundle:desktop-options.css`, 111 A dedicated script entry, such `bundle:desktop-options.css`,
126 should point at the entry scss point, and output in the _skin_ folder; 112 should simply use the `bash:css` script, passing along
113 the source file and the target.
114
115 ```js
116 {
117 // example of a new bundle for the ./css/source.scss file
118 "bundle:target.css": "npm run bash:css ./css/source.scss ./skin/target.css"
119 }
120 ```
127 121
128 In case there are dependencies, please ensure these are 122 In case there are dependencies, please ensure these are
129 imported via `@import "dep.scss"` and not via `url(...)` syntax. 123 imported via `@import "dep.scss"` and not via `url(...)` syntax.
130 124
125 As it is for JS bundles, the main `bundle` script should include each
126 CSS bundle too.
127
131 Watching 128 Watching
132 -------- 129 --------
133 130
134 While developing, it is convenient to bundle automatically 131 While developing, it is convenient to bundle automatically
135 each time a source file is changed. 132 each time a source file is changed.
136 133
137 As a team, we agreed on restructuring JS code inside the js folder, 134 As a team, we agreed on restructuring JS code inside the js folder,
138 so that is watched, and each bundled created, every time there is a changes. 135 so that is watched, and each bundled created, every time there is a changes.
139 136
140 Similarly done for bundles, watches follow the following convention: 137 Similarly done for bundles, watches follow the following convention:
(...skipping 64 matching lines...) Expand 10 before | Expand all | Expand 10 after
205 errors when adding new filters on the options page 202 errors when adding new filters on the options page
206 * `blockedURLs`: a comma-separated list of URLs that should be considered 203 * `blockedURLs`: a comma-separated list of URLs that should be considered
207 blocked (necessary to test the check for blocked scripts in sharing buttons). 204 blocked (necessary to test the check for blocked scripts in sharing buttons).
208 * `downloadStatus`: sets downloadStatus parameter for filter lists, can be used 205 * `downloadStatus`: sets downloadStatus parameter for filter lists, can be used
209 to trigger various filter list download errors 206 to trigger various filter list download errors
210 * `platform=chromium`: shows the opt-out for the developer tools panel 207 * `platform=chromium`: shows the opt-out for the developer tools panel
211 * `showNotificationUI=true`: simulates user having opted-out of notifications 208 * `showNotificationUI=true`: simulates user having opted-out of notifications
212 209
213 210
214 [crowdin]: https://crowdin.com 211 [crowdin]: https://crowdin.com
OLDNEW
« no previous file with comments | « no previous file | package.json » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld