Closed Bug 1196155 Opened 5 years ago Closed 5 years ago

Some ESLint rules have been removed in v1.1.0

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox44 fixed)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- fixed

People

(Reporter: pbro, Assigned: jryans)

Details

Attachments

(1 file)

I recently upgraded ESLint to 1.1.0 and I now see the following 2 linting errors on any devtools files:

Rule 'no-empty-class' was removed and replaced by: no-empty-character-class
Rule 'spaced-line-comment' was removed and replaced by: spaced-comment

We should update browser/devtools/.eslintrc to reflect these changes *and* send an email to everyone asking to upgrade their eslint install!

Or have it auto-update via mach? mach eslint landed a month ago or so, it's worth looking into how that works and see if there's a n update mechanism in place.
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Comment on attachment 8663918 [details]
MozReview Request: Bug 1196155 - Update renamed ESLint rules. r=pbrosset

https://reviewboard.mozilla.org/r/19865/#review17973

Thanks for taking care of this Ryan.
Of course this begs the question: how are we going to deal with the fact that people may have various versions of ESLint installed?
I think the answer could be: first of all, let's document which version we use on https://wiki.mozilla.org/DevTools/CodingStandards
Then, let's try to maybe stick to this version, at least for a while, until we have a compelling reason to move to another one.
Mike's working on something like `./mach eslint-setup` which would install ESLint locally, this would solve this.
Attachment #8663918 - Flags: review?(pbrosset) → review+
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #2)
> Mike's working on something like `./mach eslint-setup` which would install
> ESLint locally, this would solve this.

mach eslint --setup

I am starting to think it should be part of mach bootstrap though although that would mean implementing the Windows version (mach bootstrap is osx & Linux only).
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #2)
> Comment on attachment 8663918 [details]
> MozReview Request: Bug 1196155 - Update renamed ESLint rules. r=pbrosset
> 
> https://reviewboard.mozilla.org/r/19865/#review17973
> 
> Thanks for taking care of this Ryan.
> Of course this begs the question: how are we going to deal with the fact
> that people may have various versions of ESLint installed?
> I think the answer could be: first of all, let's document which version we
> use on https://wiki.mozilla.org/DevTools/CodingStandards
> Then, let's try to maybe stick to this version, at least for a while, until
> we have a compelling reason to move to another one.
> Mike's working on something like `./mach eslint-setup` which would install
> ESLint locally, this would solve this.

Okay, for now I've noted "ESLint 1.1.0 or later" on the wiki.  The latest version is 1.5.0, not sure if we want to just say that or not.

Some Node projects install ESLint locally (instead of globally) and invoke via node_modules/.bin/eslint to ensure the expected version is used.  I don't really feel strongly about what approach we take.

I did a try run, though I don't think we test this stuff yet...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=be970ca11aaf
https://hg.mozilla.org/mozilla-central/rev/2acca26772aa
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> Okay, for now I've noted "ESLint 1.1.0 or later" on the wiki.  The latest
> version is 1.5.0, not sure if we want to just say that or not.
> 
> Some Node projects install ESLint locally (instead of globally) and invoke
> via node_modules/.bin/eslint to ensure the expected version is used.  I
> don't really feel strongly about what approach we take.
> 

That would complicate adding modules... especially when future plugins may have a minimum version. We are best to just recommend the latest version.

> I did a try run, though I don't think we test this stuff yet...
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=be970ca11aaf

We don't test it yet... we need to make our codebase more conformant first.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.