Last Comment Bug 1325464 - Enable object-shorthand eslint rules for toolkit
: Enable object-shorthand eslint rules for toolkit
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: General (show other bugs)
: unspecified
: Unspecified Unspecified
-- normal (vote)
: mozilla53
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
:
Mentors:
Depends on: 1326305
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-22 12:53 PST by Jared Wein [:jaws] (please needinfo? me)
Modified: 2016-12-30 18:48 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
Bug 1325464 - Use ES6 method syntax for preferences. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for applications.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for content.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for main.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for security.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for subdialogs.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for sync.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for search.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Use ES6 method syntax for privacy.js. (59 bytes, text/x-review-board-request)
2016-12-22 13:35 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled. (59 bytes, text/x-review-board-request)
2016-12-28 09:34 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review
Bug 1325464 - Manually fix the remaining ES6 method syntax eslint errors. (59 bytes, text/x-review-board-request)
2016-12-28 09:34 PST, Jared Wein [:jaws] (please needinfo? me)
MattN+bmo: review+
Details | Review

Description User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 12:53:01 PST

    
Comment 1 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 2 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 3 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 4 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 5 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 6 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 7 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 8 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 9 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 10 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:35:52 PST Comment hidden (mozreview-request)
Comment 11 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-22 13:36:57 PST
Apologies if there are too many commits, I'm getting on the 'hg wip', 'hg histedit', 'firefoxtrees' train and off of the MQ train.
Comment 12 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:47:21 PST
Comment on attachment 8821305 [details]
Bug 1325464 - Use ES6 method syntax for preferences.

https://reviewboard.mozilla.org/r/100582/#review101428
Comment 13 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:50:23 PST
Comment on attachment 8821306 [details]
Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled.

https://reviewboard.mozilla.org/r/100584/#review101430

::: browser/components/preferences/in-content/advanced.js:21
(Diff revision 1)
> -  init: function()
> +  init()
>    {

If you want to do another bug to move the braces to the same line I won't complain :)
Comment 14 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:51:21 PST
Comment on attachment 8821307 [details]
Bug 1325464 - Use ES6 method syntax for applications.js.

https://reviewboard.mozilla.org/r/100586/#review101432
Comment 15 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:51:52 PST
Comment on attachment 8821308 [details]
Bug 1325464 - Use ES6 method syntax for content.js.

https://reviewboard.mozilla.org/r/100588/#review101434
Comment 16 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:52:28 PST
Comment on attachment 8821309 [details]
Bug 1325464 - Use ES6 method syntax for main.js.

https://reviewboard.mozilla.org/r/100590/#review101436
Comment 17 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:52:55 PST
Comment on attachment 8821310 [details]
Bug 1325464 - Use ES6 method syntax for security.js.

https://reviewboard.mozilla.org/r/100592/#review101438
Comment 18 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:53:32 PST
Comment on attachment 8821311 [details]
Bug 1325464 - Use ES6 method syntax for subdialogs.js.

https://reviewboard.mozilla.org/r/100594/#review101440
Comment 19 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:54:08 PST
Comment on attachment 8821312 [details]
Bug 1325464 - Use ES6 method syntax for sync.js.

https://reviewboard.mozilla.org/r/100596/#review101442
Comment 20 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:54:43 PST
Comment on attachment 8821313 [details]
Bug 1325464 - Use ES6 method syntax for search.js.

https://reviewboard.mozilla.org/r/100598/#review101444
Comment 21 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-23 15:55:55 PST
Comment on attachment 8821314 [details]
Bug 1325464 - Use ES6 method syntax for privacy.js.

https://reviewboard.mozilla.org/r/100600/#review101446

::: browser/components/preferences/in-content/privacy.js:29
(Diff revision 1)
>  
>    /**
>     * Show the Tracking Protection UI depending on the
>     * privacy.trackingprotection.ui.enabled pref, and linkify its Learn More link
>     */
> -  _initTrackingProtection: function() {
> +  _initTrackingProtection() {

Please enforce this with eslint: http://eslint.org/docs/rules/object-shorthand

It even supports `--fix` so you can make sure you didn't miss anything.
Comment 22 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 23 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 24 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 25 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 26 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 27 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 28 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 29 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 30 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 31 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 32 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 33 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:34:36 PST Comment hidden (mozreview-request)
Comment 34 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-28 09:40:21 PST
The last two patches I have uploaded are in response to comment #21. I have expanded the scope of this bug to fix all of /toolkit and /browser. https://reviewboard.mozilla.org/r/101196/diff/#index_header was auto-generated using --fix.
Comment 35 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 08:12:59 PST Comment hidden (mozreview-request)
Comment 36 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 08:12:59 PST Comment hidden (mozreview-request)
Comment 37 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 08:17:04 PST
Comment on attachment 8821306 [details]
Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled.

https://reviewboard.mozilla.org/r/100584/#review101430

> If you want to do another bug to move the braces to the same line I won't complain :)

Yeah, I'll fix that in another bug. Hopefully with an eslint rule to help keep us consistent.
Comment 38 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-29 10:00:05 PST
Comment on attachment 8822216 [details]
Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled.

https://reviewboard.mozilla.org/r/101196/#review101896

This looks fine to me but I wonder if there should be a chance for people to voice why we should potentially wait on this as it may break some older tools (e.g. editor functionality to jump to a method definition?).

::: browser/base/content/aboutDialog-appUpdater.js:164
(Diff revision 2)
>        return Services.prefs.getBoolPref("app.update.auto");
>      }
>      catch (e) { }
>      return true; // Firefox default is true
>    },
>  

Your commit message seems to have two messages folded together which should be fixed. If you're using histedit you could have used `roll` instead of `fold` to avoid that.

::: toolkit/components/captivedetect/captivedetect.js:338
(Diff revision 2)
>      };
>  
>      this._runningRequest['urlFetcher'] = urlFetcher;
>    },
>  
>    _startLogin: function _startLogin() {

`--fix` understandably doesn't seem to touch cases where the function name matches the property name like this so it's possible that this patch is introducing inconsitency of shorthand vs. longhand with a function name in the same object but in those cases there was already inconsitency of longhand without vs. with a function name so I think that's okay.
Comment 39 User image Matthew N. [:MattN] (PM if requests are blocking you) 2016-12-29 10:04:47 PST
Comment on attachment 8822217 [details]
Bug 1325464 - Manually fix the remaining ES6 method syntax eslint errors.

https://reviewboard.mozilla.org/r/101198/#review101900
Comment 40 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 10:56:30 PST Comment hidden (mozreview-request)
Comment 41 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 10:56:30 PST Comment hidden (mozreview-request)
Comment 42 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:01:32 PST
Comment on attachment 8822216 [details]
Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled.

https://reviewboard.mozilla.org/r/101196/#review101896

SublimeText3 supports it and I see that VisualStudio also supports it. I'm not sure what other tools people may be using, but we've had usages of it in our code since they were introduced around Firefox33/34 so I'm OK with pushing this forward.
Comment 43 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 44 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 45 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 46 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 47 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 48 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 49 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 50 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 51 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 52 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 53 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 54 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 11:46:46 PST Comment hidden (mozreview-request)
Comment 55 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 12:28:22 PST Comment hidden (mozreview-request)
Comment 56 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 12:28:22 PST Comment hidden (mozreview-request)
Comment 57 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 12:28:22 PST Comment hidden (mozreview-request)
Comment 58 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 13:01:28 PST Comment hidden (mozreview-request)
Comment 59 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 13:01:28 PST Comment hidden (mozreview-request)
Comment 60 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 13:01:28 PST Comment hidden (mozreview-request)
Comment 61 User image Pulsebot 2016-12-29 13:02:57 PST
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4079437c4648
Use ES6 method syntax for preferences. r=MattN
https://hg.mozilla.org/integration/autoland/rev/cd10db6087dd
Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled. r=MattN
https://hg.mozilla.org/integration/autoland/rev/562ddc32cc21
Manually fix the remaining ES6 method syntax eslint errors. r=MattN
Comment 62 User image Wes Kocher (:KWierso) 2016-12-29 14:06:11 PST
I had to back these out for xpcshell failures like https://treeherder.mozilla.org/logviewer.html#?job_id=65324637&repo=autoland

https://hg.mozilla.org/integration/autoland/rev/6fb777f48fa6
Comment 63 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 15:39:02 PST Comment hidden (mozreview-request)
Comment 64 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-29 15:39:02 PST Comment hidden (mozreview-request)
Comment 65 User image Pulsebot 2016-12-29 15:40:50 PST
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/01d6a34ee366
Use ES6 method syntax for preferences. r=MattN
https://hg.mozilla.org/integration/autoland/rev/15ea2e33fcd4
Enable object-shorthand rule and run 'mach eslint --fix' with the rule enabled. r=MattN
https://hg.mozilla.org/integration/autoland/rev/e551e215986a
Manually fix the remaining ES6 method syntax eslint errors. r=MattN
Comment 67 User image Jared Wein [:jaws] (please needinfo? me) 2016-12-30 18:48:06 PST
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #37)
> Comment on attachment 8821306 [details]
> Bug 1325464 - Enable object-shorthand rule and run 'mach eslint --fix' with
> the rule enabled.
> 
> https://reviewboard.mozilla.org/r/100584/#review101430
> 
> > If you want to do another bug to move the braces to the same line I won't complain :)
> 
> Yeah, I'll fix that in another bug. Hopefully with an eslint rule to help
> keep us consistent.

Filed bug 1326511 for this work.

Note You need to log in before you can comment on or make changes to this bug.