As part of rolling out ESLint across the tree, we should enable it for dom/notification/test/unit/ There's background on our eslint setups here: https://developer.mozilla.org/docs/ESLint Here's some approximate steps: - In .eslintignore, remove the `dom/notification/test/unit/**` line. - Run eslint: ./mach eslint --fix dom/notification/test/unit/ This should fix some of the issues. - Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok. - Create a commit of the work so far. - For the remaining issues, you'll need to fix them by hand. I think most of these will be no-undef. The best way to fix those will be to do a repository move for common_test_notificationdb.js to head_notificationdb.js, and change the name in the xpcshell.ini file. This will allow ESLint to automatically pick up the symbols that the file is declaring and use them in the test files. (you can just run `./mach eslint dom/notification/test/unit/` to check you've fixed them). - Create a second commit of the manual changes and push it to mozreview: http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html If you're not sure of how to fix something, please look at the rules documentation http://eslint.org/docs/rules/ or just ask here.

Comment on attachment 8938586 [details] Bug 1423843 - Enable ESLint for dom/notification/test/unit/ https://reviewboard.mozilla.org/r/209228/#review215108 Thank you for the patch, it is a great start, and fixes the ESLint issues! One minor thing, is that I'm not seeing the move of dom/notification/test/unit/common_test_notificationdb.js to dom/notification/test/unit/head_notificationdb.js in the patch. Did you `hg mv dom/notification/test/unit/common_test_notificationdb.js dom/notification/test/unit/head_notificationdb.js` or something like that? There's also a couple more comments to address as well. Once you've addressed them and push the new patch, if you can also come into mozreview and mark them as fixed, that'd be useful in tidying up. Also, please can you add mccr8 to the review request as well please, as its his area (change `r?standard8` to `r?standard8,r?mccr8`). Any questions, please ask. Thanks ::: .eslintignore (Diff revision 1) > # generated or library files in pocket > browser/extensions/pocket/content/panels/js/tmpl.js > browser/extensions/pocket/content/panels/js/vendor/** > # generated or library files in activity-stream > browser/extensions/activity-stream/data/content/activity-stream.bundle.js > -browser/extensions/activity-stream/test/** Please undo this change, we still need it (I suspect it is a merge/rebase that went bad). ::: .eslintignore:411 (Diff revision 1) > toolkit/mozapps/update/tests/data/xpcshellConstantsPP.js > > # Third party > toolkit/modules/third_party/** > third_party/** > + All the files seemed to have gained an extra newline at the end. This is unusual, maybe your editor is misconfigured or something?

Hi Nicola, are you planning to finish this?

Opening up as mentored bug. As there's a few things that have changed, I'd suggest following comment 0 from scratch against the latest tree, rather than trying to reuse the previous patch. However, note that the reviewers should be ":standard8" and ":mccr8". Also note that we now use phabricator rather than mozreview, see here for info: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

Hey, I am beginner. Can I take this up?

@akshitsoni0000, sure please do. We'll assign once the first patch is posted.

Mark I have submitted both commits. Please look into them. Thanks

Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dd0480856174 Enabled ESLint for dom/notification/test/unit r=Standard8,mccr8