1342734 - Remove legacy generator from calendar/providers/gdata/.

Status: RESOLVED FIXED

(Calendar :: Provider: GData, defect, P3)

Description

[Tooru Fujisawa [:arai]]

Now gdata can use ES6 generator.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Remove legacy generator from calendar/providers/gdata/.

changed legacy generator to ES6 generator,
and also replaced `throw new Task.Result(x)` to `return x`.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8841300 [details] [diff] [review]
Remove legacy generator from calendar/providers/gdata/.

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

Thanks for the patch, looks good!

::: calendar/providers/gdata/modules/gdataSession.jsm
@@ +373,5 @@
>          }
>      },
>  
>      asyncPaginatedRequest: function(aRequest, onFirst, onEach, onLast) {
> +        return Task.spawn(function*() {

Maybe you can turn some of these into using Task.async() instead?

Comment 3

[Tooru Fujisawa [:arai]]

Thank you for reviewing :)

(In reply to Philipp Kewisch [:Fallen] from comment #2)
> Comment on attachment 8841300 [details] [diff] [review]
> Remove legacy generator from calendar/providers/gdata/.
> 
> Review of attachment 8841300 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the patch, looks good!
> 
> ::: calendar/providers/gdata/modules/gdataSession.jsm
> @@ +373,5 @@
> >          }
> >      },
> >  
> >      asyncPaginatedRequest: function(aRequest, onFirst, onEach, onLast) {
> > +        return Task.spawn(function*() {
> 
> Maybe you can turn some of these into using Task.async() instead?

yes, it would also work (while the diff becomes bigger, to reduce indentation)

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

(In reply to Tooru Fujisawa [:arai] from comment #3)
> Thank you for reviewing :)
> yes, it would also work (while the diff becomes bigger, to reduce
> indentation)

I don't mind this, if you want you could also attach a patch with the -w option. arai, are you still interested in making this change or should we push what you have (both is fine with me) ?

Comment 5

[Tooru Fujisawa [:arai]]

sorry, I forgot to update depending bug field.
I was about to continue working on this (and bug 1340947) after bug 1342828 fixed, since I couldn't verify this patch without that


(In reply to Philipp Kewisch [:Fallen] from comment #4)
> (In reply to Tooru Fujisawa [:arai] from comment #3)
> > Thank you for reviewing :)
> > yes, it would also work (while the diff becomes bigger, to reduce
> > indentation)
> 
> I don't mind this, if you want you could also attach a patch with the -w
> option. arai, are you still interested in making this change or should we
> push what you have (both is fine with me) ?

I'm going to apply the change.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Thanks for the quick update!

Comment 7

[Martin Schröder [:mschroeder]]

All tests should run in a (try)build again, so do you plan to continue work on this issue?

Comment 8

[Tooru Fujisawa [:arai]]

thanks.
I'm a bit busy now, I think I can continue from next week.

Comment 9

[Tooru Fujisawa [:arai]]

try run with updated patch:
  https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=dd72badcd03f179b3b44cac65e0636cc1437ccf8

result on trunk for comparison:
  https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=6b0c4e39b0d680dee1cfa688bbf92b1ab8907566&filter-searchStr=linux%2064

Comment 10

[Martin Schröder [:mschroeder]]

Philipp asked three months ago if you want to use Task.async() in some places (comment#2) which you agreed to (comment#5). Is the try run with the patch attached to this bug or with an updated patch? We can also simply check in the existing patch and open a new bug for the remaining work.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Remove legacy generator from calendar/providers/gdata/. r=Fallen

here's the updated patch that uses Task.async, and this is for the try run.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7953ac6987eb
Remove legacy generator from calendar/providers/gdata/. r=philipp