Closed Bug 1501931 Opened 6 years ago Closed 6 years ago

Enable ESLint for parser/

Categories

(Core :: DOM: HTML Parser, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: standard8, Assigned: r2hkri, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [lang=js])

Attachments

(2 files, 1 obsolete file)

As part of rolling out ESLint across the tree, we should enable it for the parser/ directories. I'm happy to mentor this bug. There's background on our eslint setups here: https://developer.mozilla.org/docs/ESLint Please note: We want to end up with two separate commits. One with the automatic changes, the second with the manual changes. It helps with reviewing if these appear as a commit series in phabricator. Here's some approximate steps: - In .eslintignore, remove the `parser/**` line. - Run eslint: ./mach eslint --fix parser This should fix most/all 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, e.g. $ hg commit -m "Bug nnn - Enable ESLint for parser/ (automatic changes)" parser/ - For the remaining issues, you'll need to fix them by hand. Most of those should be reasonably easy to understand, but there's more information on some specific bits (especially no-undef) here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/ESLint#Common_issues_and_how_to_solve_them http://eslint.org/docs/rules/ (you can just run `./mach eslint parser` to check you've fixed them). - Create a second commit of the manual changes, e.g. $ hg commit -m "Bug nnn - Enable ESLint for parser/ (manual changes)" - Post the two commits via phabricator: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html When you post, please request initial review from myself (`standard8`), I'll then redirect to the owners of the code if the changes look good.
Priority: -- → P3
Having a look at this one
Assignee: nobody → riccardo.bellingeri
Riccardo has told me he's no longer working on this, so opening up again.
Assignee: riccardo.bellingeri → nobody
I would like to try this, thanks!
(In reply to r2hkri from comment #4) > I would like to try this, thanks! Hi, please do. Feel free to ask here or on irc if you have any questions (https://wiki.mozilla.org/Irc #introduction).
Assignee: nobody → r2hkri
I'm running into a problem where a variable is defined in an (x)html file and then referenced inside a loaded script. For example: https://searchfox.org/mozilla-central/source/parser/htmlparser/tests/mochitest/test_bug688580.html#23 https://searchfox.org/mozilla-central/source/parser/htmlparser/tests/mochitest/file_bug688580.js#2 I tried the suggestions from https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/ESLint#no-undef and none of them worked or fit the case. I also can't do something like /* globals state */ without also disabling the no-global-assign rule.
Depends on D11920
Attachment #9026442 - Attachment is obsolete: true
Blake, could you have a look at the automatic changes as well please. Although Phabricator is saying "needs revision", I think it is currently confused...
Flags: needinfo?(mrbkap)
Sorry, I misread the Phabricator queue.
Flags: needinfo?(mrbkap)
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f456b3dd43c0 Enable ESLint for parser/ (automatic fixes) r=Standard8,mrbkap https://hg.mozilla.org/integration/autoland/rev/7c02d6c28133 Enable ESLint for parser/ (manual fixes) r=Standard8,mrbkap
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: