1228792 - Add-ons Manager should pass eslint

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Robert Helmer [:rhelmer]]

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.

Comment 1

[Robert Helmer [:rhelmer]]

(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*

Comment 2

[Dave Townsend [:mossop]]

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.

Comment 3

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

bug 1228792 - quote non-numerals r?mossop

Comment 4

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - use function* for generators r?mossop

bug 1228792 - use function* for generators r?mossop

Comment 5

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - use standard version of catch r?mossop

bug 1228792 - use standard version of catch r?mossop

Comment 6

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

bug 1228792 - remove addons manager from eslint ignore r?mossop

Comment 7

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

Comment 8

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

bug 1228792 - remove use of array comprehensions r?mossop

Comment 9

[Robert Helmer [:rhelmer]]

(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.

Comment 10

[Robert Helmer [:rhelmer]]

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/

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 8693388 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

https://reviewboard.mozilla.org/r/26427/#review23987

Comment 12

[Dave Townsend [:mossop]]

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 13

[Dave Townsend [:mossop]]

Comment on attachment 8693390 [details]
MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

https://reviewboard.mozilla.org/r/26431/#review24009

Comment 14

[Dave Townsend [:mossop]]

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 15

[Dave Townsend [:mossop]]

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()

Comment 16

[Robert Helmer [:rhelmer]]

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/

Comment 17

[Robert Helmer [:rhelmer]]

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/

Comment 18

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

bug 1228792 - use function* for generators r?mossop

Comment 19

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

bug 1228792 - use standard version of catch r?mossop

Comment 20

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

bug 1228792 - remove addons manager from eslint ignore r?mossop

Comment 21

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

Comment 22

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove use of array comprehensions r?mossop

bug 1228792 - remove use of array comprehensions r?mossop

Comment 23

[Robert Helmer [:rhelmer]]

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/

Comment 24

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - use standard version of catch r?mossop

bug 1228792 - remove addons manager from eslint ignore r?mossop

Comment 25

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - use function* for generators r?mossop

bug 1228792 - move addons manager preprocessor instructions to AppConstants.jsm r?mossop

Comment 26

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

bug 1228792 - remove use of array comprehensions r?mossop

Comment 27

[Dave Townsend [:mossop]]

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

Comment 28

[Robert Helmer [:rhelmer]]

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.

Comment 29

[Robert Helmer [:rhelmer]]

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

> Brackets need to extend to surround the e too!

sigh

Comment 30

[Robert Helmer [:rhelmer]]

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/

Comment 31

[Robert Helmer [:rhelmer]]

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/

Comment 32

[Robert Helmer [:rhelmer]]

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/

Comment 33

[Robert Helmer [:rhelmer]]

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/

Comment 34

[Robert Helmer [:rhelmer]]

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/

Comment 35

[Robert Helmer [:rhelmer]]

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 36

[Robert Helmer [:rhelmer]]

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.

Comment 37

[Robert Helmer [:rhelmer]]

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.

Comment 38

[Robert Helmer [:rhelmer]]

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.

Comment 39

[Robert Helmer [:rhelmer]]

(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.

Comment 40

[Dave Townsend [:mossop]]

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 41

[Dave Townsend [:mossop]]

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 !

Comment 42

[Dave Townsend [:mossop]]

Comment on attachment 8693869 [details]
MozReview Request: bug 1228792 - use function* for generators r?mossop

https://reviewboard.mozilla.org/r/26597/#review24167

Comment 43

[Robert Helmer [:rhelmer]]

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 44

[Robert Helmer [:rhelmer]]

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 45

[Robert Helmer [:rhelmer]]

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 46

[Robert Helmer [:rhelmer]]

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 47

[Robert Helmer [:rhelmer]]

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/

Comment 48

[Robert Helmer [:rhelmer]]

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 49

[Robert Helmer [:rhelmer]]

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 50

[Robert Helmer [:rhelmer]]

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 51

[Robert Helmer [:rhelmer]]

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 52

[Robert Helmer [:rhelmer]]

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 53

[Dave Townsend [:mossop]]

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?

Comment 54

[Dave Townsend [:mossop]]

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 55

[Robert Helmer [:rhelmer]]

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 56

[Robert Helmer [:rhelmer]]

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 57

[Robert Helmer [:rhelmer]]

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 58

[Robert Helmer [:rhelmer]]

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 59

[Robert Helmer [:rhelmer]]

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/

Comment 60

[Robert Helmer [:rhelmer]]

Tests pass for me locally, also pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=acd639049aba

Comment 61

[Robert Helmer [:rhelmer]]

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 62

[Robert Helmer [:rhelmer]]

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/

Comment 63

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove addons manager from eslint ignore r?mossop

bug 1228792 - remove addons manager from eslint ignore r?mossop

Comment 64

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - use standard version of catch r?mossop

bug 1228792 - use standard version of catch r?mossop

Comment 65

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - use function* for generators r?mossop

bug 1228792 - use function* for generators r?mossop

Comment 66

[Robert Helmer [:rhelmer]]

Attachment: MozReview Request: bug 1228792 - remove leading 0, be explicit about octals r?mossop

bug 1228792 - remove leading 0, be explicit about octals r?mossop

Comment 67

[Robert Helmer [:rhelmer]]

Carrying forward previous r+ from mossop, waiting on new try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42c7c03cfff0

Comment 68

[Robert Helmer [:rhelmer]]

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 69

[Robert Helmer [:rhelmer]]

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/

Comment 70

[Robert Helmer [:rhelmer]]

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/

Comment 71

[Robert Helmer [:rhelmer]]

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/

Comment 72

[Robert Helmer [:rhelmer]]

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/

Comment 73

[Robert Helmer [:rhelmer]]

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.

Comment 74

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/a405a78a5229
https://hg.mozilla.org/integration/fx-team/rev/07267b96720a
https://hg.mozilla.org/integration/fx-team/rev/9abfe93bfd41
https://hg.mozilla.org/integration/fx-team/rev/5ada25e1f319
https://hg.mozilla.org/integration/fx-team/rev/e5c4090f0c4d

Comment 75

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a405a78a5229
https://hg.mozilla.org/mozilla-central/rev/07267b96720a
https://hg.mozilla.org/mozilla-central/rev/9abfe93bfd41
https://hg.mozilla.org/mozilla-central/rev/5ada25e1f319
https://hg.mozilla.org/mozilla-central/rev/e5c4090f0c4d