SyntaxError: yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead) @ chrome://browser/content/places/controller.js:939 See this code block, 929 // Do removal in chunks to give some breath to main-thread. 930 function pagesChunkGenerator(aURIs) { 931 while (aURIs.length) { 932 let URIslice = aURIs.splice(0, REMOVE_PAGES_CHUNKLEN); 933 PlacesUtils.bhistory.removePages(URIslice, URIslice.length); 934 Services.tm.mainThread.dispatch(function() { 935 try { 936 gen.next(); 937 } catch (ex if ex instanceof StopIteration) {} 938 }, Ci.nsIThread.DISPATCH_NORMAL); 939 yield; 940 } http://hg.mozilla.org/mozilla-central/annotate/c3598b276048/browser/components/places/content/controller.js#l939

Hey Marco! I guess this bug is trivial to start. Could you tell me how do I go about cloning the repository?

Is this sufficient or do I need to change anything else as well? Regards, Sushant

Comment on attachment 768499 [details] [diff] [review] v1: Changed yield to yield undefined Review of attachment 768499 [details] [diff] [review]: ----------------------------------------------------------------- it looks enough for this case, thanks. Actually, if you would wish to fix other cases in Places, I could find other 2 instances of the problem: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/browser/browser_colorAnalyzer.js#24 http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unit/test_isURIVisited.js#90 I don't care about further reviewing those test changes, so feel free to just do those and push. You should also fix the commit message to actually include the bug number and the review flag

oh and sorry if I missed the question in comment 1 I'd like to suggest you to use the needinfo? flag that is at the end of the bug report to ask direct questions to developers, that is easier for us to track. you can find most of the information on how to contribute at https://developer.mozilla.org/en-US/docs/Introduction but feel free to ask if you have any doubt. you can also reach us on irc.mozilla.org in channels #developers and #fx-team for any questions

(In reply to Marco Bonardo [:mak] from comment #3) > Comment on attachment 768499 [details] [diff] [review] > v1: Changed yield to yield undefined > > Review of attachment 768499 [details] [diff] [review]: > ----------------------------------------------------------------- > > it looks enough for this case, thanks. > Actually, if you would wish to fix other cases in Places, I could find other > 2 instances of the problem: > http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/ > tests/browser/browser_colorAnalyzer.js#24 > http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/ > tests/unit/test_isURIVisited.js#90 > I don't care about further reviewing those test changes, so feel free to > just do those and push. > > You should also fix the commit message to actually include the bug number > and the review flag I'm not sure if I can push directly. I don't have commit access as of now. So should I submit a patch for those 2 files in this bug only?

yes, you can add those other 2 changes to the existing patch, and attach an updated version with all of them here. For check-in you can refer to https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F and once the patch is in the right shape, just add the checkin-needed keyword to the above keywords field, and some sheriff will take care of pushing your patch to the repository.

Hey Marco, I've added those changes some time back. I had proposed them to you for a review. Could you have a look and then somebody can checkin for me! Regards, Sushant

(In reply to Sushant Hiray [:sushant] from comment #7) > Created attachment 768518 [details] [diff] [review] > Bug:887320 Changed deprecated yield to yeild undefined Hi Sushant, In comment #4, Marco already said, it's fine to just change these two lines without further review. However, you will need to change the summary of the patch to include the bug number and the reviewer. So, please make sure that the patch header has this information (Bug 887320, r=mak) in addition to the description. Then attach the new patch, set the review flag to '+', and add 'checkin-needed' to the keywords field, please. :-) Thank you!

(In reply to Sushant Hiray [:sushant] from comment #10) > Created attachment 770287 [details] [diff] [review] > Bug:887320 r=mak {Changed yield to yield undefined} OK, I was unclear, I'm afraid. If you open the attachment, you will see these lines at the top: # HG changeset patch # Parent 110d0591ce68008ac68ca6da0c8b7c06e169130f # User Sushant Hiray <hiraysushant@gmail.com> modifying deprecated yield without value to yield undefined The bottom line should be something along the lines of: Bug 887320 - yield without a value is deprecated, use yield undefined, r=mak If you're using mercurial queues, you can use "hg qref -e" to do that. For plain mercurial commits, I'm not sure - you'd have to remove the commit (hg strip can do that) and recommit with the correct message (hg commit -m "..."), I expect. Finally, don't set the checkin flag on the attachment. Once you've added the patch, back on the bug page, there's a "Keywords" field, near the top. Add 'checkin-needed' in there.

(In reply to :Gijs Kruitbosch from comment #11) > (In reply to Sushant Hiray [:sushant] from comment #10) > > Created attachment 770287 [details] [diff] [review] > > Bug:887320 r=mak {Changed yield to yield undefined} > > OK, I was unclear, I'm afraid. If you open the attachment, you will see > these lines at the top: > > # HG changeset patch > # Parent 110d0591ce68008ac68ca6da0c8b7c06e169130f > # User Sushant Hiray <hiraysushant@gmail.com> > modifying deprecated yield without value to yield undefined > > > The bottom line should be something along the lines of: > > Bug 887320 - yield without a value is deprecated, use yield undefined, r=mak > > > If you're using mercurial queues, you can use "hg qref -e" to do that. For > plain mercurial commits, I'm not sure - you'd have to remove the commit (hg > strip can do that) and recommit with the correct message (hg commit -m > "..."), I expect. > > Finally, don't set the checkin flag on the attachment. Once you've added the > patch, back on the bug page, there's a "Keywords" field, near the top. Add > 'checkin-needed' in there. Ooh I see, Thanks a lot for your help! I was unaware of the checkin-needed in "Keywords" field! Thanks for stopping by and helping! I hope this patch fulfills all the guidelines.

Sorry for late answer, I was traveling and got hijacked by higher priority stuff. But I see Gijs has done a great job at replacing me! Thanks you both, and congratulations on your first patch.

Thanks a lot :-)