Closed Bug 1330014 Opened 3 years ago Closed 3 years ago

Enable eslint for /services

Categories

(Cloud Services :: General, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
Tracking Status
firefox53 --- fixed

People

(Reporter: jaws, Assigned: jaws)

Details

Attachments

(2 files)

/services is not mentioned in .eslintignore but there is no .eslintrc.js file in /services.

Adding one shows ~4083 errors. Running eslint --fix brings this down to 405 errors.
Attachment #8825455 - Flags: review?(markh)
Comment on attachment 8825455 [details]
Bug 1330014 - Add .eslintrc.js file to /services and run eslint --fix to automatically fix most of the errors. --fix brings the error count down from 4083 to 321 errors.

https://reviewboard.mozilla.org/r/103604/#review104352

phew! I also wish we were removing cloudsync instead of fixing it, but...

::: services/cloudsync/tests/xpcshell/test_bookmarks.js:18
(Diff revision 1)
>  
>  }
>  
> -add_task(function* test_merge_bookmarks_flat () {
> +add_task(function* test_merge_bookmarks_flat() {
>    try {
>  	  let rootFolder = yield CloudSync().bookmarks.getRootFolder("TEST");

this file still has a mix of tabs and spaces, which wouldn't hurt fixing while we are here.

::: services/common/kinto-http-client.js:17
(Diff revision 1)
>   * See the License for the specific language governing permissions and
>   * limitations under the License.
>   */
>  
>  /*
>   * This file is generated from kinto-http.js - do not modify directly.

ISTM we should skip this file and add it to the ignored list?

::: services/common/kinto-offline-client.js:17
(Diff revision 1)
>   * See the License for the specific language governing permissions and
>   * limitations under the License.
>   */
>  
>  /*
>   * This file is generated from kinto.js - do not modify directly.

ditto

::: services/common/rest.js:105
(Diff revision 1)
>      Ci.nsIBadCertListener2,
>      Ci.nsIInterfaceRequestor,
>      Ci.nsIChannelEventSink
>    ]),
>  
> -  /*** Public API: ***/
> +  /** * Public API: ***/

huh - lint complaining about this seems a little heavy handed :(

::: services/fxaccounts/FxAccountsClient.jsm:91
(Diff revision 1)
>     *          verified (optional): flag indicating verification status of the
>     *                               email
>     *        }
>     */
> -  _createSession: function(path, email, password, getKeys=false,
> -                           retryOK=true) {
> +  _createSession(path, email, password, getKeys = false,
> +                           retryOK = true) {

this indentation looks odd

::: services/fxaccounts/FxAccountsManager.jsm:264
(Diff revision 1)
>     *      the current account OUT.
>     *   2) The person typing can't prove knowledge of the password used
>     *      to log in. Failure should do nothing.
>     */
> -  _refreshAuthentication: function(aAudience, aEmail, aPrincipal,
> -                                   logoutOnFailure=false) {
> +  _refreshAuthentication(aAudience, aEmail, aPrincipal,
> +                                   logoutOnFailure = false) {

odd indentation here too
Attachment #8825455 - Flags: review?(markh) → review+
Comment on attachment 8825455 [details]
Bug 1330014 - Add .eslintrc.js file to /services and run eslint --fix to automatically fix most of the errors. --fix brings the error count down from 4083 to 321 errors.

https://reviewboard.mozilla.org/r/103604/#review104920

I wonder if we should exclude the mozmill directories - i.e. is that actually imported code?

Other than that, it looks good.

Thanks!

::: services/common/blocklist-clients.js:305
(Diff revision 1)
>          // sync to fail. We will accumulate telemetry for such failures in bug
>          // 1254099.
>        }
>      }
>    } else {
> -    return;
> +

Should probably drop the now-empty else block here.
Attachment #8825455 - Flags: review?(standard8) → review+
Comment on attachment 8825456 [details]
Bug 1330014 - Remove extra lines that eslint --fix adds for no-useless-return.

https://reviewboard.mozilla.org/r/103606/#review104938
Attachment #8825456 - Flags: review?(standard8) → review+
Comment on attachment 8825455 [details]
Bug 1330014 - Add .eslintrc.js file to /services and run eslint --fix to automatically fix most of the errors. --fix brings the error count down from 4083 to 321 errors.

https://reviewboard.mozilla.org/r/103604/#review104352

> this file still has a mix of tabs and spaces, which wouldn't hurt fixing while we are here.

This will get fixed as part of enabling the "no-mixed-spaces-and-tabs" rule, so I don't want to do that here and now.

> huh - lint complaining about this seems a little heavy handed :(

This is fixed in the manual follow-up. It's an error due to the "spaced-comment" rule which says that there should be a blank line before a comment starts, ie "//this is a comment" is wrong and should be "// this is a comment". In cases like the one above that are done for styling, it complains for the wrong reasons. I balanced out the asterisks in the manual follow-up patch.
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7e0a0bd74199
Add .eslintrc.js file to /services and run eslint --fix to automatically fix most of the errors. --fix brings the error count down from 4083 to 321 errors. r=markh,standard8
https://hg.mozilla.org/integration/autoland/rev/afe70927277c
Remove extra lines that eslint --fix adds for no-useless-return. r=standard8
Sorry had to back out for xpcshell test_storage_manager.js failures, i.e., https://treeherder.mozilla.org/logviewer.html#?job_id=68646097&repo=autoland&lineNumber=2390
Flags: needinfo?(jaws)
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e9a029b62c4
Backed out changeset afe70927277c for xpcshell test_storage_manager.js failures
https://hg.mozilla.org/integration/autoland/rev/33c475c8f360
Backed out changeset 7e0a0bd74199
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4a4bd6ca18c3
Add .eslintrc.js file to /services and run eslint --fix to automatically fix most of the errors. --fix brings the error count down from 4083 to 321 errors. r=markh,standard8
https://hg.mozilla.org/integration/autoland/rev/9677f72cddc9
Remove extra lines that eslint --fix adds for no-useless-return. r=standard8
You need to log in before you can comment on or make changes to this bug.