1027246 - Use ES6 generators (function*) instead of old spidermonkey generators (without *)

Status: RESOLVED DUPLICATE of bug 887895

(DevTools :: General, defect)

Description

[vikneshwar]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.114 Safari/537.36

Comment 1

[vikneshwar]

Replace fuction with function* for generators in

1)http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/stylesheets.js

2)http://dxr.mozilla.org/mozilla-central/source/browser/devtools/projecteditor/lib/stores/local.js

3)http://dxr.mozilla.org/mozilla-central/source/browser/devtools/shadereditor/shadereditor.js

Comment 2

[vikneshwar]

Replace function with function* for generators in

1)http://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/stylesheets.js

2)http://dxr.mozilla.org/mozilla-central/source/browser/devtools/projecteditor/lib/stores/local.js

3)http://dxr.mozilla.org/mozilla-central/source/browser/devtools/shadereditor/shadereditor.js

Comment 3

[vikneshwar]

Attachment: generators.patch

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8442332 [details] [diff] [review]
generators.patch

Review of attachment 8442332 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good :)

Try push: https://tbpl.mozilla.org/?tree=Try&rev=0b052237f30a

Comment 5

[Ryan VanderMeulen [:RyanVM]]

That Try push looks really orange....

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Vikneshwar, did you run the tests locally? Can you reproduce the failures? You likely have a syntax error or something.

Comment 7

[Ian Moody [:Kwan] (UTC+1)]

Attachment: rebased generators.patch

Rebased patch, review carried forward

Comment 8

[Ian Moody [:Kwan] (UTC+1)]

Attachment: generatorReturn.patch

Seemed a shame for vikneshwar's patch to go unused.  All it needed was the "throw new Task.Result()"s replacing with simple returns.
 	
https://treeherder.mozilla.org/#/jobs?repo=try&revision=06ddec3c3a31

Comment 9

[Ian Moody [:Kwan] (UTC+1)]

Let's get this bit in before it bit-rots again.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/46cfe0a6aa6c
https://hg.mozilla.org/integration/fx-team/rev/f4240674fd6c

Comment 11

[Carsten Book [:Tomcat]]

had to be backedout first for the suspicion of causing a dt test bustage but turned out to be innocent and so relanded as

remote:   https://hg.mozilla.org/integration/fx-team/rev/087d9b31dca4
remote:   https://hg.mozilla.org/integration/fx-team/rev/cc4794a3d869

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/087d9b31dca4
https://hg.mozilla.org/mozilla-central/rev/cc4794a3d869

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

I think this can be closed, yeah?

Comment 14

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I think we still haven't removed all the old generators, so I think it should stay open.

Comment 15

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I believe we completed this in bug 887895 as part of starting to use ESLint, since it failed to parse functions with yield without *.

Patrick should know for sure.

Comment 16

[Patrick Brosset <:pbro>]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #15)
> I believe we completed this in bug 887895 as part of starting to use ESLint,
> since it failed to parse functions with yield without *.
Yes we did, so we should close this bug I believe.
The thing is we're, as of now, running eslint automatically as part of a build process, so nothing prevents people from re-introducing spidermonkey-only syntax (and in fact this has happened already a couple of times), and this usually goes un-noticed at review time.
However, because a lot of us run eslint as part of our editor, this is usually discovered pretty quickly and fixed.

Comment 17

[Patrick Brosset <:pbro>]

we're *not* running eslint automatically is what I meant to say.

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Okay, then I believe we can consider this bug completed by bug 887895.

Even though it's still possible to add one again, we at least removed them all once, and that was task this bug aimed for.

Comment 19

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.