Fix method name misspelling in MediaControlService.java

RESOLVED FIXED in Firefox 55

Status

()

P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: walkingice, Assigned: sumangpt9, Mentored)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(fennec+, firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [good first bug][lang=java])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
In MediaControlService.java, there is a private method with misspelling. Please fix from 'Lollopop' to 'Lollipop'
(Reporter)

Comment 1

2 years ago
This is a easy bug and good for any newcomer who is interesting in Fennec development. Feel free to pick up this one.

You can refer bug 1324656 to get more hints.

Once complete a patch, to let it be an attachment to this bug, or send to MozReview[1].

[1] http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html
Mentor: walkingice0204
tracking-fennec: --- → +

Comment 2

2 years ago
I am interested in fixing this bug.

Comment 3

2 years ago
Created attachment 8822830 [details] [diff] [review]
Fix method name misspelling in MediaControlService.java
Attachment #8822830 - Flags: review?(walkingice0204)
Comment hidden (mozreview-request)
Hi, I've submitted the fix for review https://reviewboard.mozilla.org/r/102072/ I'm not able to add reviewers though. When I give Julian Chu and Sebastian Kaspari's mail ID, it says user does not exist. Can you please tell me how to proceed?
Attachment #8823543 - Flags: review?(s.kaspari)
Attachment #8823543 - Attachment is obsolete: true
Attachment #8823543 - Flags: review?(s.kaspari)
Comment hidden (mozreview-request)
Attachment #8823547 - Flags: review?(walkingice0204)
Attachment #8823547 - Flags: review?(s.kaspari)
Comment on attachment 8823547 [details]
Fix for bug 1326374

Julian can review this, I'll clear the review flag for myself.
Attachment #8823547 - Flags: review?(s.kaspari)
(Reporter)

Updated

2 years ago
Attachment #8822830 - Flags: review?(walkingice0204) → review+
(Reporter)

Comment 8

2 years ago
Comment on attachment 8822830 [details] [diff] [review]
Fix method name misspelling in MediaControlService.java

Mohammad, the fix itself is good. However I see a warning 'Unassigned bug with patches attached' from Bugzilla. Could you please update your patch with comment, then Bugzilla won't argue for this. And next time when you fix complex problem, good comment helps a lot.
Attachment #8822830 - Flags: review+
(Reporter)

Comment 9

2 years ago
Sivaranjani, I appreciate your effort on this. Your patch and using-mozreview are great to me. Since Mohammad already created a valid patch, I would like to accept Mohammad's patch once it been updated.

I think you already have enough knowledge of development flow, maybe you will be interesting in another bugs? If so, you can find any bugs which be flagged as 'tracking-fennec+'[1].

[1] https://bugzilla.mozilla.org/buglist.cgi?f1=cf_blocking_fennec&o1=equals&query_format=advanced&v1=%2B&product=Firefox%20for%20Android

Comment 10

2 years ago
Created attachment 8823960 [details] [diff] [review]
MediaControlService.patch

Updated the patch with comments.
Attachment #8822830 - Attachment is obsolete: true
Attachment #8823960 - Flags: review?(walkingice0204)
(In reply to Julian Chu [:walkingice] from comment #9)
> Sivaranjani, I appreciate your effort on this. Your patch and
> using-mozreview are great to me. Since Mohammad already created a valid
> patch, I would like to accept Mohammad's patch once it been updated.
> 
> I think you already have enough knowledge of development flow, maybe you
> will be interesting in another bugs? If so, you can find any bugs which be
> flagged as 'tracking-fennec+'[1].
> 
> [1]
> https://bugzilla.mozilla.org/buglist.
> cgi?f1=cf_blocking_fennec&o1=equals&query_format=advanced&v1=%2B&product=Fire
> fox%20for%20Android

Yes, sure. Thank you!
(Reporter)

Updated

2 years ago
Assignee: nobody → mohammad14147
(Reporter)

Comment 12

2 years ago
Mohammad, the warning is my bad, and I fixed it by assigning this bug to you. I pushed[1] you patch to try-server to run tests.

To make you patch be able to merge, please provide hg-friendly patch. If you use mecurial, you can generate patch by `hg export` command. If you use git, you can configure your environment then send changes to MozReview, then you don't need to attach patch file to here.

The link in comment #1 describes how to configure your development environment to achieve this.


Feel free to ask question.

Comment 14

2 years ago
I am not able to create hg-friendly patch, I have tried many ways and I am getting problems such as on using `hg export` (https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial) I am getting some patch file created but I don't get whether it is the correct patch file or not and when I am trying to use mozReview for review by `hg push review` I am getting something like this 

''' redirecting push to https://reviewboard-hg.mozilla.org/gecko
(ignoring public changeset 7652a58efa46 in review request)
abort: no non-public changesets left to review
(add or change the -r argument to include draft changesets) '''

after this I tried `hg push -r 7652a58efa46 review` but then also the same result.

And on using `rbt post` command it shows there are no outgoing changes.

Plz help me in this problem. I am executing all commands inside `mozilla-central` directory.
(Reporter)

Comment 15

2 years ago
This bug is very easy(as you did in previous patch), so I don't think you will create more than just one patch file. As your description, you created  "some" patch files, I guess you didn't use hg properly. Maybe you can try to understand

* How to check commit log in hg
* How to write single good(comment) patch in hg
* How to export a patch file by specific commit

"7652a58efa46" is existing commit in hg repository. (try "hg export -r 7652a58efa46"), apparently that is not what you want to push to MozReview. It seems you don't really understand what you did to hg. I think learning *hg* is valuable since it is important tool in Mozilla. You can easily find guides to learn Mercurial then try again.
(Reporter)

Comment 16

2 years ago
Mohammad, are you still working on this?
Flags: needinfo?(mohammad14147)

Comment 17

2 years ago
Hey, I think no one is working on it currently. Can I fix this?
(Reporter)

Comment 18

2 years ago
Srivatsav, thank you for be interesting in this. Sivaranjani already create a patch[1] for this. And I would like to accept that patch in the end of this week, if I can't get response from Mohammad.

[1] https://reviewboard.mozilla.org/r/102082/diff/1#index_header
(Reporter)

Comment 19

2 years ago
mozreview-review
Comment on attachment 8823547 [details]
Fix for bug 1326374

https://reviewboard.mozilla.org/r/102082/#review111044
Attachment #8823547 - Flags: review?(walkingice0204) → review+
(Reporter)

Updated

2 years ago
Keywords: checkin-needed
(Reporter)

Comment 20

2 years ago
Comment on attachment 8823960 [details] [diff] [review]
MediaControlService.patch

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

Reviewed another patch from MozReview
Attachment #8823960 - Flags: review?(walkingice0204)
seems sebastian need to give final r+ here for autolander
Flags: needinfo?(s.kaspari)
Keywords: checkin-needed
(Reporter)

Updated

2 years ago
Assignee: mohammad14147 → ranjani4ever
Flags: needinfo?(mohammad14147)
Flags: needinfo?(s.kaspari)
Keywords: checkin-needed
Keywords: checkin-needed
(Reporter)

Updated

2 years ago
Flags: needinfo?(ranjani4ever)
Yes, will make the necessary changes in the commit message.
Comment hidden (mozreview-request)

Comment 26

2 years ago
mozreview-review
Comment on attachment 8841333 [details]
Bug 1326374 Changed the method spelling from Lollopop to lollipop.

https://reviewboard.mozilla.org/r/115588/#review117768
Attachment #8841333 - Flags: review?(s.kaspari) → review+
(Reporter)

Comment 27

2 years ago
mozreview-review
Comment on attachment 8841333 [details]
Bug 1326374 Changed the method spelling from Lollopop to lollipop.

https://reviewboard.mozilla.org/r/115588/#review119462
Attachment #8841333 - Flags: review?(walkingice0204) → review+
(Reporter)

Comment 28

2 years ago
Since Suman already sent a valid patch and granted r+, I would like to re-assign this bug to Suman. I also appreciate Sivaranjani's concern of this bug.
Assignee: ranjani4ever → sumangpt9
Flags: needinfo?(ranjani4ever)
(Reporter)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 29

2 years ago
(In reply to Julian Chu [:walkingice] from comment #28)
> Since Suman already sent a valid patch and granted r+, I would like to
> re-assign this bug to Suman. I also appreciate Sivaranjani's concern of this
> bug.

Thanks Julian! Next, I would need a walk through on how to test my patch on try server.
(Assignee)

Comment 30

2 years ago
(In reply to suman gupta from comment #29)
> (In reply to Julian Chu [:walkingice] from comment #28)
> > Since Suman already sent a valid patch and granted r+, I would like to
> > re-assign this bug to Suman. I also appreciate Sivaranjani's concern of this
> > bug.
> 
> Thanks Julian! Next, I would need a walk through on how to test my patch on
> try server.

Julian, I can see the patch passing all the runs on try. So what am I supposed to do next to complete the patch submission process?

Comment 31

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/341ec10ef4c9
Changed the method spelling from Lollopop to lollipop. r=sebastian,walkingice
Keywords: checkin-needed
Priority: -- → P3

Comment 32

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/341ec10ef4c9
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
status-firefox53: --- → wontfix
status-firefox54: --- → wontfix
You need to log in before you can comment on or make changes to this bug.