Some ESLint rules have been removed in v1.1.0

RESOLVED FIXED in Firefox 44

Status

RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: pbro, Assigned: jryans)

Tracking

unspecified
Firefox 44

Firefox Tracking Flags

(firefox44 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

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)

Updated

3 years ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED
(Assignee)

Comment 1

3 years ago
Created attachment 8663918 [details]
MozReview Request: Bug 1196155 - Update renamed ESLint rules. r=pbrosset

Bug 1196155 - Update renamed ESLint rules. r=pbrosset
Attachment #8663918 - Flags: review?(pbrosset)
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).
(Assignee)

Comment 4

3 years ago
(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
Last Resolved: 3 years ago
status-firefox44: --- → fixed
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.

Updated

2 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.