Closed Bug 1445895 Opened 3 years ago Closed 3 years ago
Utils .make URI with Services .io .new URI in Feeds .jsm
BrowserUtils.makeURI (which is deprecated) in Feeds.jsm can be replaced with Services.io.newURI: https://searchfox.org/mozilla-central/rev/8976abf9cab8eb4661665cc86bd355cd08238011/browser/modules/Feeds.jsm#77 For instructions on how to get your local build of Firefox up and running and submit your patch, see https://developer.mozilla.org/en-US/docs/Introduction. Please leave a comment if you would like to be assigned to this bug and feel free to ask questions here or via IRC if you're stuck.
Can I work on this?
(In reply to Trisha from comment #1) > Can I work on this? Hi Trisha, this bug is rather something for someone entirely new to Firefox development. We have a number of "good next bugs" that would suit you better, I believe. How about bug 1442303 or bug 1437885? Thanks!
Hi Johann , I would like to work on this !
Please review it!
Hi Shreya! Thank you for picking this up. You have to upload a patch for review, not the file you changed. Please create a patch using Mercurial and upload it. You can generate a patch after you've made changes to file(s) by using 'hg commit'. I recommend using mozReview to upload your patch. You can read all about configuring Mercurial to use mozReview here: https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install-mercurial.html#mozreview-install-mercurial Feel free to ping me on IRC if you have any questions. You can find me on the #fx-team channel.
Comment on attachment 8960992 [details] [diff] [review] Patch1.txt Hi Shreya, you can export your patch to get the diff of what you changed. What you have uploaded is just the full Feeds.jsm file. If your commit is at the tip then you can run `hg export -r tip > 1445895.patch` then you can upload the 1445895.patch file.
Hi Shreya, are you still trying to get the patch uploaded? Do you need more help with that?
Assignee: shreyasonawane54 → nobody
Status: ASSIGNED → NEW
I would like to work on this.
Thanks! Let me know if you have any questions!
Assignee: nobody → gilad.bau
Status: NEW → ASSIGNED
One small (big) question: after I made the change, how can I test this? I feel a bit lost in what exactly this jsm module does..
(In reply to gilad.bau from comment #12) > One small (big) question: after I made the change, how can I test this? I > feel a bit lost in what exactly this jsm module does.. Yeah, sorry, that module isn't exactly straightforward to understand or test. The function you're amending is used in a couple of places: https://searchfox.org/mozilla-central/search?q=isValidFeed&redirect=false One of them is the feed view in page info, so if you go to a news site that supports RSS and select Tools > Page Info, you should see a "Feeds" tab with the list of feeds. I hope that helps. When working on bugs you should usually understand what they're supposed to do, but good first bugs tend to be a little different since they're mostly about getting your environment set up. :) Thanks!
Thanks! I already found the file, but didn't know what it affected until now. I'm attaching my patch. Are you the reviewer?
Comment on attachment 8968333 [details] [diff] [review] bugfix_1445895.patch Review of attachment 8968333 [details] [diff] [review]: ----------------------------------------------------------------- Oh, sorry, I didn't notice this. For the future, if you want to get review you need to set the review? flag on patch attachments (or use mozreview https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html). Thanks for the patch, this looks great! I'm going to set the checkin-needed flag so that someone checks it in for us. Usually we would also do a try run (https://wiki.mozilla.org/ReleaseEngineering/TryServer) but this is a really simple patch and I assume you ran it locally, right? :)
Attachment #8968333 - Flags: review+
Also a note on the commit message format, the correct format is: Bug ###### - Short description of patch in imperative form. r=reviewer So in your case: Bug 1445895 - Replace BrowserUtils.makeURI with Services.io.newURI in Feeds.jsm. r=johannh Can you amend your patch with the correct message (using `hg commit --amend`) and upload it again here? Thanks!
Attachment #8968333 - Attachment is obsolete: true
Comment on attachment 8969188 [details] [diff] [review] bugfix_1445895.patch Review of attachment 8969188 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8969188 - Flags: review+
Attachment #8960992 - Attachment is obsolete: true
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/2569c92fd524 Replace BrowserUtils.makeURI with Services.io.newURI in Feeds.jsm. r=johannh
You need to log in before you can comment on or make changes to this bug.