package.json - Issue 29677659: Issue #6303 - Introduce basic foundation for automation in adblockplusui

Delta Between Two Patch Sets: package.json

Issue 29677659: Issue #6303 - Introduce basic foundation for automation in adblockplusui (Closed)

Left Patch Set: Created Jan. 23, 2018, 2:42 p.m.

Right Patch Set: Created Feb. 7, 2018, 2:32 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 {	1 {

2 "name": "adblockplusui",	2 "name": "adblockplusui",

3 "version": "0.0.0",	3 "private": true,
Thomas Greiner 2018/01/23 19:22:36 Technically, we haven't decided on how we want to Technically, we haven't decided on how we want to version UI releases so we may want to wait for that until specifying a version number. In that regard, setting "private: true" could be appropriate until we have a release process. What do you think? a.giammarchi 2018/01/24 10:52:37 makes sense Show quoted text On 2018/01/23 19:22:36, Thomas Greiner wrote: > Technically, we haven't decided on how we want to version UI releases so we may > want to wait for that until specifying a version number. In that regard, setting > "private: true" could be appropriate until we have a release process. > > What do you think? makes sense
4 "description": "AsBlock Plus UI",	4 "description": "Adblock Plus UI",
Thomas Greiner 2018/01/23 19:22:35 Typo: Replace "AsBlock Plus UI" with "Adblock Plus Typo: Replace "AsBlock Plus UI" with "Adblock Plus UI" BTW we could use the text "Shared Adblock Plus UI code" which we use at https://github.com/adblockplus/adblockplusui and https://hg.adblockplus.org/adblockplusui/ if you want. a.giammarchi 2018/01/24 10:52:37 AsBlock ... gosh, 1 word, 2 errors ... coffee here Show quoted text On 2018/01/23 19:22:35, Thomas Greiner wrote: > Typo: Replace "AsBlock Plus UI" with "Adblock Plus UI" > > BTW we could use the text "Shared Adblock Plus UI code" which we use at > https://github.com/adblockplus/adblockplusui and > https://hg.adblockplus.org/adblockplusui/ if you want. AsBlock ... gosh, 1 word, 2 errors ... coffee here I come again!
5 "scripts": {	5 "scripts": {

6 "lint": "npm run lint:css && npm run lint:js",	6 "lint": "npm run lint:js",

7 "lint:css": "stylelint ./skin",
Thomas Greiner 2018/01/23 19:22:35 Detail: You're specifying ".js" for JavaScript li Detail: You're specifying ".js" for JavaScript linting so is there a reason why you haven't appended ".css" here? a.giammarchi* 2018/01/24 10:52:37 skin has no subfolders while eslint was going thro Show quoted text On 2018/01/23 19:22:35, Thomas Greiner wrote: > Detail: You're specifying ".js" for JavaScript linting so is there a reason why > you haven't appended ".css" here? skin has no subfolders while eslint was going through the whole project, including node_modules folders. This is why ./.js is mandatory for eslint, but ./lib and ./ext won't probably need the .js specificity. Accordingly, either I use ./.js only for the project root to avoid parsing sub folders, and drop .js for lib and ext, or I add .css to stylelint command. I don't have strong preferences here, let me know which one you prefer. Thomas Greiner 2018/01/24 11:56:33 The latter sounds most consistent in that case (i. Show quoted text On 2018/01/24 10:52:37, a.giammarchi wrote: > On 2018/01/23 19:22:35, Thomas Greiner wrote: > > Detail: You're specifying ".js" for JavaScript linting so is there a reason > why > > you haven't appended ".css" here? > > skin has no subfolders while eslint was going through the whole project, > including node_modules folders. This is why ./.js is mandatory for eslint, but > ./lib and ./ext won't probably need the .js specificity. > > Accordingly, either I use ./.js only for the project root to avoid parsing sub > folders, and drop .js for lib and ext, or I add .css to stylelint command. > > I don't have strong preferences here, let me know which one you prefer. The latter sounds most consistent in that case (i.e. adding ".css" to the stylelint command) due to the limitation with "./.js".
8 "lint:js": "eslint ./.js ./lib/.js ./ext/*.js",	7 "lint:js": "eslint ./.js ./lib/.js ./ext/*.js",

9 "prepare": "python2 ensure_dependencies.py",	8 "prepare": "python2 ensure_dependencies.py",

10 "start": "http-server $1"	9 "start": "http-server"

11 },	10 },

12 "devDependencies": {	11 "devDependencies": {

13 "eslint": "^4.16.0",	12 "eslint": "^4.16.0",

14 "eslint-config-eyeo": "^2.0.0",	13 "eslint-config-eyeo": "^2.0.0",

15 "eslint-config-medikoo-es5": "^1.4.7",	14 "http-server": "^0.11.1"

16 "eslint-config-standard": "^11.0.0-beta.0",

17 "eslint-plugin-import": "^2.8.0",

18 "eslint-plugin-node": "^5.2.1",

19 "eslint-plugin-promise": "^3.6.0",

20 "eslint-plugin-standard": "^3.0.1",
Thomas Greiner 2018/01/23 19:22:35 I get that we need both "eslint" and "eslint-confi I get that we need both "eslint" and "eslint-config-eyeo" but it'd be great if you could elaborate on why you considered the other ones necessary. While I assume that you got valid reasons for using each of them, I'd like to make sure that we're not including dependencies which we don't actually need. a.giammarchi 2018/01/24 10:52:36 Good point. I've tried `eslint` as command and it Show quoted text On 2018/01/23 19:22:35, Thomas Greiner wrote: > I get that we need both "eslint" and "eslint-config-eyeo" but it'd be great if > you could elaborate on why you considered the other ones necessary. > > While I assume that you got valid reasons for using each of them, I'd like to > make sure that we're not including dependencies which we don't actually need. Good point. I've tried `eslint` as command and it incrementally complained about other modules. I start believing something went wrong with the node_folders, parsed as well, probably containing other .eslinrc here and there. I'll try to re-test all the things being sure I always use the ./.js and lib/ext folders only. Thomas Greiner* 2018/01/24 11:56:33 Thanks, I appreciate that. In case the issue still Show quoted text On 2018/01/24 10:52:36, a.giammarchi wrote: > On 2018/01/23 19:22:35, Thomas Greiner wrote: > > I get that we need both "eslint" and "eslint-config-eyeo" but it'd be great if > > you could elaborate on why you considered the other ones necessary. > > > > While I assume that you got valid reasons for using each of them, I'd like to > > make sure that we're not including dependencies which we don't actually need. > > Good point. > > I've tried `eslint` as command and it incrementally complained about other > modules. I start believing something went wrong with the node_folders, parsed as > well, probably containing other .eslinrc here and there. > > I'll try to re-test all the things being sure I always use the ./*.js and > lib/ext folders only. Thanks, I appreciate that. In case the issue still persists, feel free to share the warnings you get.
21 "http-server": "^0.11.1",
Thomas Greiner 2018/01/23 19:22:36 Since we're no longer using test_server.py, we sho Since we're no longer using test_server.py, we should remove that script from the repository. a.giammarchi 2018/01/24 10:52:37 differently from git, when you commit the lovely m Show quoted text On 2018/01/23 19:22:36, Thomas Greiner wrote: > Since we're no longer using test_server.py, we should remove that script from > the repository. differently from git, when you commit the lovely mercurial doesn't automatically flag deleted files amnd push them ... yes, I'll be happy to learn how to explicitly drop files via hg. Thomas Greiner 2018/01/24 11:56:33 It's a bit more explicit in that regard, that's tr Show quoted text On 2018/01/24 10:52:37, a.giammarchi wrote: > differently from git, when you commit the lovely mercurial doesn't automatically > flag deleted files amnd push them ... yes, I'll be happy to learn how to > explicitly drop files via hg. It's a bit more explicit in that regard, that's true. I think what you're looking for is `hg remove` or `hg addremove`.
22 "stylelint": "^8.4.0",

23 "stylelint-config-eyeo": "*",

24 "stylelint-config-recommended": "^2.0.1"
Thomas Greiner 2018/01/24 15:40:35 Since you were able to remove "eslint-config-stand Since you were able to remove "eslint-config-standard" we can probably also remove "stylelint-config-standard" now unless stylelint works differently in that regard, for some reason.
25 }	15 }

26 }	16 }

LEFT	RIGHT