Closed Bug 1228792 Opened 4 years ago Closed 4 years ago

Add-ons Manager should pass eslint

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: rhelmer, Assigned: rhelmer)

References

Details

Attachments

(5 files, 13 obsolete files)

40 bytes, text/x-review-board-request
mossop
: review+
Details
40 bytes, text/x-review-board-request
rhelmer
: review+
Details
40 bytes, text/x-review-board-request
rhelmer
: review+
Details
40 bytes, text/x-review-board-request
rhelmer
: review+
Details
40 bytes, text/x-review-board-request
rhelmer
: review+
Details
I have started working on a patch to get `toolkit/mozapps/extensions/` to pass eslint, and to remove this directory from the top-level `.eslintignore`.

The main issues I am seeing are not too surprising:

* need .eslintrc with proper rules enabled
* remove C preprocessor instructions (e.g. move #ifdefs to AppConstants)
* use of features not on standards track (Array comprehensions)

Losing array comprehensions makes me pretty sad. If we really want it we could write a rule or I think the babel-eslint parser supports it, but we're probably better off just avoiding them for now.
(In reply to Robert Helmer [:rhelmer] from comment #0)
> * need .eslintrc with proper rules enabled
> * remove C preprocessor instructions (e.g. move #ifdefs to AppConstants)
> * use of features not on standards track (Array comprehensions)

* yield used without function*
Status: NEW → ASSIGNED
Awesome, I was going to look into it myself next week. Bug 1226386 removes almost all of the preprocessing but yeah we need to get rid of the array comprehensions and stuff.
bug 1228792 - quote non-numerals r?mossop
Attachment #8693387 - Flags: review?(dtownsend)
bug 1228792 - use function* for generators r?mossop
Attachment #8693388 - Flags: review?(dtownsend)
bug 1228792 - use standard version of catch r?mossop
Attachment #8693389 - Flags: review?(dtownsend)
bug 1228792 - remove addons manager from eslint ignore r?mossop
Attachment #8693390 - Flags: review?(dtownsend)
bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop
Attachment #8693391 - Flags: review?(dtownsend)
bug 1228792 - remove use of array comprehensions r?mossop
Attachment #8693392 - Flags: review?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #2)
> Awesome, I was going to look into it myself next week. Bug 1226386 removes
> almost all of the preprocessing but yeah we need to get rid of the array
> comprehensions and stuff.

Yes I was thinking this :) I went ahead and made all the changes necessary to pass ESLint here (broken up into separate commits), but let's go ahead with landing bug 1226386 first and I'll deal with any conflicts after.
Flags: needinfo?(dtownsend)
Comment on attachment 8693392 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26435/diff/1-2/
Flags: needinfo?(dtownsend)
Comment on attachment 8693388 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

https://reviewboard.mozilla.org/r/26427/#review23987
Attachment #8693388 - Flags: review?(dtownsend) → review+
Attachment #8693389 - Flags: review?(dtownsend)
Comment on attachment 8693389 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

https://reviewboard.mozilla.org/r/26429/#review23993

::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:1615
(Diff revision 1)
> -       } catch (e if e instanceof OS.File.Error && e.becauseNoSuchFile) {
> +         } catch (e) {

You should be able to do this:

    } catch (e) {
      if (e instanceof ...) {
      } else {
        logger.error("Malformed" ...
      }
    }

::: toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm:653
(Diff revision 1)
> +        if (! e instanceof SyntaxError)

JS operator precedence makes ! more important than instanceof so you have to wrap the instanceof check in brackets.

::: toolkit/mozapps/extensions/internal/PluginProvider.jsm:467
(Diff revision 1)
> +      if (! e.result && e.result != Components.results.NS_ERROR_FAILURE)

This should be || not &&

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1806
(Diff revision 1)
> +      if (! e instanceof OS.File.Error && ! e.becauseNoSuchFile)

Same issues as above

::: toolkit/mozapps/extensions/nsBlocklistService.js:102
(Diff revision 1)
> +        ! ex.result == Cr.NS_NOINTERFACE)

This needs to be:
    (!(ex instanceof Components.Exception) || ex.result != Cr.NS_NO_INTERFACE)

::: toolkit/mozapps/extensions/test/xpcshell/head_addons.js:1078
(Diff revision 1)
> +      if (! ex instanceof OS.File.Error)

Ditto

::: toolkit/mozapps/extensions/test/xpcshell/test_mapURIToAddonID.js:271
(Diff revision 1)
> -    catch (ex if ex.result) {
> +      catch (ex) {

See previous comment about not needing to nest catches.
Comment on attachment 8693390 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

https://reviewboard.mozilla.org/r/26431/#review24009
Attachment #8693390 - Flags: review?(dtownsend) → review+
Attachment #8693391 - Flags: review?(dtownsend)
Comment on attachment 8693391 [details]
MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

https://reviewboard.mozilla.org/r/26433/#review24013

I'll wait to review this after rebasing on top of bug 1226386
Comment on attachment 8693392 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

https://reviewboard.mozilla.org/r/26435/#review24015

Using array.filter and array.map can reduce the ammount of additional code here. I've commented on a few to show examples but I think most of this can be done in the same way. Where you've actually got an iterator and not a real array then Array.from can be used to convert.

::: toolkit/mozapps/extensions/AddonManager.jsm:1303
(Diff revision 2)
> -      port.sendAsyncMessage("PluginList", [filterProperties(p) for (p of aPlugins)]);
> +      let plugins = [];

You can just do:

    aPlugins.map(filterProperties)

::: toolkit/mozapps/extensions/AddonManager.jsm:2437
(Diff revision 2)
> -    let promises = [for (p of this.providers) promiseCallProvider(p, "getAddonByID", aID)];
> +    let promises = [];

this.providers.map(p => promiseCallProvider(p, "getAddonByID", aID))

::: toolkit/mozapps/extensions/content/update.js:216
(Diff revision 2)
> -    gUpdateWizard.addons = [a for (a of fetchedAddons) if (a)];
> +    gUpdateWizard.addons = [];

fetchedAddons.filter(a => a)

::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:640
(Diff revision 2)
> -    let ids = [a.id for (a of allAddons)];
> +    let ids = [];

let ids = allAddons.map(a => a.id)

::: toolkit/mozapps/extensions/internal/AddonRepository_SQLiteMigrator.jsm:63
(Diff revision 2)
> -      let resultArray = [addon for ([,addon] of Iterator(results))];
> +      let resultArray = [];

Iterator is going to go away soon too:

    let resultArray = results.values()
Attachment #8693392 - Flags: review?(dtownsend)
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/1-2/
Attachment #8693387 - Attachment description: MozReview Request: bug 1228792 - quote non-numerals r?mossop → MozReview Request: bug 1228792 - use standard version of catch r?mossop
Attachment #8693390 - Attachment is obsolete: true
Attachment #8693391 - Attachment is obsolete: true
Attachment #8693392 - Attachment is obsolete: true
Attachment #8693388 - Attachment is obsolete: true
Attachment #8693389 - Attachment is obsolete: true
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/2-3/
Attachment #8693387 - Attachment description: MozReview Request: bug 1228792 - use standard version of catch r?mossop → MozReview Request: bug 1228792 - quote non-numerals r?mossop
bug 1228792 - use function* for generators r?mossop
Attachment #8693863 - Flags: review?(dtownsend)
bug 1228792 - use standard version of catch r?mossop
Attachment #8693864 - Flags: review?(dtownsend)
bug 1228792 - remove addons manager from eslint ignore r?mossop
Attachment #8693865 - Flags: review?(dtownsend)
bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop
Attachment #8693866 - Flags: review?(dtownsend)
bug 1228792 - remove use of array comprehensions r?mossop
Attachment #8693867 - Flags: review?(dtownsend)
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26587/diff/1-2/
Attachment #8693865 - Attachment is obsolete: true
Attachment #8693865 - Flags: review?(dtownsend)
Attachment #8693866 - Attachment is obsolete: true
Attachment #8693866 - Flags: review?(dtownsend)
Attachment #8693867 - Attachment is obsolete: true
Attachment #8693867 - Flags: review?(dtownsend)
Attachment #8693863 - Flags: review?(dtownsend) → review+
bug 1228792 - remove addons manager from eslint ignore r?mossop
Attachment #8693868 - Flags: review?(dtownsend)
bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop
Attachment #8693869 - Flags: review?(dtownsend)
bug 1228792 - remove use of array comprehensions r?mossop
Attachment #8693870 - Flags: review?(dtownsend)
Attachment #8693868 - Flags: review?(dtownsend) → review+
Attachment #8693869 - Flags: review?(dtownsend)
Attachment #8693870 - Flags: review?(dtownsend)
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

https://reviewboard.mozilla.org/r/26587/#review24097

::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:1614
(Diff revision 2)
> -       } catch (e if e instanceof OS.File.Error && e.becauseNoSuchFile) {
> +       if (e instanceof OS.File.Error && e.becauseNoSuchFile) {

Perhaps I typed badly, you still need a catch block around the if/else block here.

::: toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm:653
(Diff revision 2)
> +        if (! e (instanceof SyntaxError))

Brackets need to extend to surround the e too!

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1806
(Diff revision 2)
> +      if (! e (instanceof OS.File.Error) || ! e.becauseNoSuchFile)

Brace must surround e

::: toolkit/mozapps/extensions/test/xpcshell/head_addons.js:1078
(Diff revision 2)
> +      if (!ex (instanceof OS.File.Error))

Brace must surround ex
Attachment #8693864 - Flags: review?(dtownsend)
https://reviewboard.mozilla.org/r/26587/#review24097

> Perhaps I typed badly, you still need a catch block around the if/else block here.

Nope, what you said made sense - I just dropped the line accidentally.
https://reviewboard.mozilla.org/r/26587/#review24097

> Brackets need to extend to surround the e too!

sigh
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/3-4/
Attachment #8693387 - Attachment description: MozReview Request: bug 1228792 - quote non-numerals r?mossop → MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop
Comment on attachment 8693863 [details]
MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26585/diff/1-2/
Attachment #8693863 - Attachment description: MozReview Request: bug 1228792 - use function* for generators r?mossop → MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop
Attachment #8693863 - Flags: review+ → review?(dtownsend)
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26587/diff/2-3/
Attachment #8693864 - Attachment description: MozReview Request: bug 1228792 - use standard version of catch r?mossop → MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop
Attachment #8693864 - Flags: review?(dtownsend)
Comment on attachment 8693868 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26595/diff/1-2/
Attachment #8693868 - Attachment description: MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop → MozReview Request: bug 1228792 - use standard version of catch r?mossop
Attachment #8693868 - Flags: review+ → review?(dtownsend)
Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26597/diff/1-2/
Attachment #8693869 - Attachment description: MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop → MozReview Request: bug 1228792 - use function* for generators r?mossop
Attachment #8693869 - Flags: review?(dtownsend)
Attachment #8693870 - Attachment description: MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop → MozReview Request: bug 1228792 - quote non-numerals r?mossop
Attachment #8693870 - Flags: review?(dtownsend)
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26599/diff/1-2/
Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

I just rebased against fx-team, this one isn't quite ready yet.
Attachment #8693869 - Flags: review?(dtownsend)
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

I just rebased against fx-team, this one isn't quite ready yet.
Attachment #8693870 - Flags: review?(dtownsend)
Attachment #8693869 - Flags: review?(dtownsend)
Attachment #8693863 - Flags: review?(dtownsend)
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

(discussed in IRC) Going to use the new ES6 octal notation for the chmod settings, and remove the leading 0 for the Date parts, to make eslint happy hear but make our intent clearer.
Attachment #8693387 - Flags: review?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #27)
> Comment on attachment 8693864 [details]
> 
> ::: toolkit/mozapps/extensions/internal/AddonUpdateChecker.jsm:653
> (Diff revision 2)
> > +        if (! e (instanceof SyntaxError))
> 
> Brackets need to extend to surround the e too!
> 
> ::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1806
> (Diff revision 2)
> > +      if (! e (instanceof OS.File.Error) || ! e.becauseNoSuchFile)
> 
> Brace must surround e
> 
> ::: toolkit/mozapps/extensions/test/xpcshell/head_addons.js:1078
> (Diff revision 2)
> > +      if (!ex (instanceof OS.File.Error))
> 
> Brace must surround ex

This was a search/replace fail.
Attachment #8693864 - Flags: review?(dtownsend) → review+
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

https://reviewboard.mozilla.org/r/26587/#review24163
Comment on attachment 8693868 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

https://reviewboard.mozilla.org/r/26595/#review24165

::: toolkit/mozapps/extensions/internal/PluginProvider.jsm:505
(Diff revision 2)
> +      if (! e.result || e.result != Components.results.NS_ERROR_FAILURE)

Nit: No space after the !

::: toolkit/mozapps/extensions/nsBlocklistService.js:101
(Diff revision 2)
> +    if (! (ex instanceof Components.Exception) ||

Nit: No space after the !
Attachment #8693868 - Flags: review?(dtownsend) → review+
Attachment #8693869 - Flags: review?(dtownsend) → review+
Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

https://reviewboard.mozilla.org/r/26597/#review24167
Attachment #8693387 - Flags: review?(dtownsend)
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/4-5/
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26587/diff/3-4/
Comment on attachment 8693868 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26595/diff/2-3/
Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26597/diff/2-3/
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26599/diff/2-3/
Attachment #8693870 - Attachment description: MozReview Request: bug 1228792 - quote non-numerals r?mossop → MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop
Attachment #8693870 - Flags: review?(dtownsend)
Attachment #8693863 - Attachment is obsolete: true
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/5-6/
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26587/diff/4-5/
Comment on attachment 8693868 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26595/diff/3-4/
Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26597/diff/3-4/
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26599/diff/3-4/
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

https://reviewboard.mozilla.org/r/26599/#review24393

::: toolkit/mozapps/extensions/test/browser/browser_sorting.js:32
(Diff revision 4)
> -    size: 0265,
> +    size: "0265",

Just use 265 here

::: toolkit/mozapps/extensions/test/browser/browser_sorting.js:38
(Diff revision 4)
> -    name: "\u010Cesk\u00FD slovn\u00EDk", // Český slovník
> +    name: "\u010Cesk\u00FD slovn\u00EDk", // ČeskÃœ slovnÃ

Looks like your editor clobbered the international characters here?
Attachment #8693870 - Flags: review?(dtownsend) → review+
Attachment #8693387 - Flags: review?(dtownsend) → review+
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

https://reviewboard.mozilla.org/r/26425/#review24399

::: toolkit/mozapps/extensions/content/selectAddons.js:129
(Diff revision 6)
> -    var addons = [gAddons[id] for (id in gAddons)];
> +    let addons = gAddons.map(a => a.id);

This isn't right. I think you just want Object.values(gAddons)

::: toolkit/mozapps/extensions/internal/AddonRepository.jsm:627
(Diff revision 6)
> -    allAddons = [a for (a of allAddons) if (a.id != AddonManager.hotfixID)];
> +    allAddons = allAddons.filter(a => a != AddonManager.hotfixID);

Looks like there should be an a.id in here

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:3620
(Diff revision 6)
> -            + [for (b of bootstrapDescriptors) b] + ")");
> +            + Array.from(bootstrapDescriptors).map(a => a) + ")");

This map call doesn't do anything

::: toolkit/mozapps/extensions/internal/XPIProviderUtils.js:417
(Diff revision 6)
> -  return [for (addon of addonDB.values()) if (aFilter(addon)) addon];
> +  return Array.from(addonDB.values()).filter(a => aFilter(a));

You can just use .filter(aFilter) here
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/6-7/
Comment on attachment 8693864 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26587/diff/5-6/
Comment on attachment 8693868 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26595/diff/4-5/
Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26597/diff/4-5/
Comment on attachment 8693870 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26599/diff/4-5/
Keywords: checkin-needed
Tests pass for me locally, also pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=acd639049aba
A handful of the try tests found a problem with the array comprehension patch, re-pushed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42c7c03cfff0
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/7-8/
Attachment #8693864 - Attachment is obsolete: true
Attachment #8693868 - Attachment is obsolete: true
Attachment #8693869 - Attachment is obsolete: true
Attachment #8693870 - Attachment is obsolete: true
bug 1228792 - remove addons manager from eslint ignore r?mossop
Attachment #8695331 - Flags: review?(dtownsend)
bug 1228792 - use standard version of catch r?mossop
Attachment #8695332 - Flags: review?(dtownsend)
bug 1228792 - use function* for generators r?mossop
Attachment #8695333 - Flags: review?(dtownsend)
bug 1228792 - remove leading 0, be explicit about octals r?mossop
Attachment #8695334 - Flags: review?(dtownsend)
Attachment #8695331 - Flags: review?(dtownsend) → review+
Attachment #8695333 - Flags: review?(dtownsend) → review+
Attachment #8695334 - Flags: review?(dtownsend) → review+
Attachment #8695332 - Flags: review?(dtownsend) → review+
Carrying forward previous r+ from mossop, waiting on new try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42c7c03cfff0
Keywords: checkin-needed
Comment on attachment 8693387 [details]
MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26425/diff/8-9/
Comment on attachment 8695331 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27069/diff/1-2/
Attachment #8695331 - Flags: review+ → review?(dtownsend)
Comment on attachment 8695332 [details]
MozReview Request: bug 1228792 - use standard version of catch r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27071/diff/1-2/
Attachment #8695332 - Flags: review+ → review?(dtownsend)
Comment on attachment 8695333 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27073/diff/1-2/
Attachment #8695333 - Flags: review+ → review?(dtownsend)
Comment on attachment 8695334 [details]
MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27075/diff/1-2/
Attachment #8695334 - Flags: review+ → review?(dtownsend)
Attachment #8695331 - Flags: review?(dtownsend) → review+
Attachment #8695332 - Flags: review?(dtownsend) → review+
Attachment #8695333 - Flags: review?(dtownsend) → review+
Attachment #8695334 - Flags: review?(dtownsend) → review+
Carrying forward r+ from mossop, got a good try run for this set:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=53c9d4821a55

The test timeouts I see there don't seem to be related to this patch, and have open intermittent bugs.
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.