Closed
Bug 1368967
Opened 7 years ago
Closed 7 years ago
Enable the ESLint generator-star-spacing rule across mozilla-central
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement, P2)
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: standard8, Assigned: stevea1, Mentored)
References
Details
(Whiteboard: [lang=js])
Attachments
(1 file)
generator-star-spacing is part of ESLint style related rules to help make code more consistent. http://eslint.org/docs/rules/generator-star-spacing It is already in use for some areas, so we should expand it further. Although we're starting to move away from a lot of generators, I think it is still useful for consistency for the remaining instances. https://dxr.mozilla.org/mozilla-central/search?q=generator-star-spacing&redirect=false I'm willing to mentor this bug. Approximate steps: - Ensure you've got ESLint setup on mozilla-central (https://developer.mozilla.org/docs/ESLint). - In tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js un-comment the generator-star-spacing line. - Remove the generator-star-spacing lines from the other .eslintrc.js files (* apart from devtools/ *) - Run: ./mach eslint - You should be able to fix the failing instances automatically with: ./mach eslint --fix or maybe ./mach eslint --fix <directory> - Check the diff in case there's indentation that needs adjusting. - Commit to a patch and push it to mozreview (http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html)
Comment 1•7 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #0) > Although we're starting to move away from a lot of generators, I think it is > still useful for consistency for the remaining instances. You may want to wait until I've ran my Task.jsm removal scripts on the rest of the tree before fixing this throughout the tree.
Reporter | ||
Comment 2•7 years ago
|
||
Ok, dropping it from the mentored list for now.
Comment 3•7 years ago
|
||
(In reply to Florian Quèze [:florian] [:flo] from comment #1) > (In reply to Mark Banner (:standard8) from comment #0) > > > Although we're starting to move away from a lot of generators, I think it is > > still useful for consistency for the remaining instances. > > You may want to wait until I've ran my Task.jsm removal scripts on the rest > of the tree before fixing this throughout the tree. This was referring to bug 1374282 which is now fixed.
Reporter | ||
Updated•7 years ago
|
Priority: -- → P2
Reporter | ||
Comment 4•7 years ago
|
||
We can start moving this forward again - see comment 0 for details.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
I followed the steps in comment #0 (including avoiding the devtools/ files) and completed the clean-up. Patch posted...
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → stevea1
Reporter | ||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8904113 [details] Bug 1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central. https://reviewboard.mozilla.org/r/175890/#review180908 Thank you for the patch Steve, it looks good, there's just one minor bit to fix up, then we'll be good to go. ::: tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js:116 (Diff revision 1) > > // No spaces between function name and parentheses > "func-call-spacing": "error", > > // Require function* name() > - // "generator-star-spacing": ["error", {"before": false, "after": true}], > + "generator-star-spacing": ["error", {"before": false, "after": true}], This isn't really your fault, the line needs to change to: "generator-star-spacing": ["error", {"after": true, "before": false}], This is due to the sort-keys rule being on the eslint-plugin-mozilla. You can re-check it with: ./mach eslint tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js
Attachment #8904113 -
Flags: review?(standard8)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
Mark, I had seen a message about "expected object keys to be in ascending order" message when I tested eslint and wasn't quite sure if I should make the change, so thanks for clarifying - definitely makes sense now. I updated the patch with corrected tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js...
Reporter | ||
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8904113 [details] Bug 1368967 - Enable the ESLint generator-star-spacing rule across mozilla-central. https://reviewboard.mozilla.org/r/175890/#review181168 Thank you for the update. Looks good. r=Standard8
Attachment #8904113 -
Flags: review?(standard8) → review+
Comment 11•7 years ago
|
||
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eb3853161334 Enable the ESLint generator-star-spacing rule across mozilla-central. r=standard8
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/eb3853161334
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•6 years ago
|
Product: Testing → Firefox Build System
Updated•5 years ago
|
Keywords: good-first-bug
Version: Version 3 → 3 Branch
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•