1326374 - Fix method name misspelling in MediaControlService.java

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Julian Chu [:walkingice]]

In MediaControlService.java, there is a private method with misspelling. Please fix from 'Lollopop' to 'Lollipop'

Comment 1

[Julian Chu [:walkingice]]

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

Comment 2

[Mohammad Nayeem]

I am interested in fixing this bug.

Comment 3

[Mohammad Nayeem]

Attachment: Fix method name misspelling in MediaControlService.java

Comment 4

[Sivaranjani Subramanian]

Attachment: Fix for bug 1326374

Review commit: https://reviewboard.mozilla.org/r/102074/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102074/

Comment 5

[Sivaranjani Subramanian]

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?

Comment 6

[Sivaranjani Subramanian]

Attachment: Fix for bug 1326374

Review commit: https://reviewboard.mozilla.org/r/102082/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102082/

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8823547 [details]
Fix for bug 1326374

Julian can review this, I'll clear the review flag for myself.

Comment 8

[Julian Chu [:walkingice]]

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.

Comment 9

[Julian Chu [:walkingice]]

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

[Mohammad Nayeem]

Attachment: MediaControlService.patch

Updated the patch with comments.

Comment 11

[Sivaranjani Subramanian]

(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!

Comment 12

[Julian Chu [:walkingice]]

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 13

[Julian Chu [:walkingice]]

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ab9133961482ce070cd4aa1806a05d899507e97

Comment 14

[Mohammad Nayeem]

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.

Comment 15

[Julian Chu [:walkingice]]

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.

Comment 16

[Julian Chu [:walkingice]]

Mohammad, are you still working on this?

Comment 17

[Srivatsav Gunisetty]

Hey, I think no one is working on it currently. Can I fix this?

Comment 18

[Julian Chu [:walkingice]]

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

Comment 19

[Julian Chu [:walkingice]]

Comment on attachment 8823547 [details]
Fix for bug 1326374

https://reviewboard.mozilla.org/r/102082/#review111044

Comment 20

[Julian Chu [:walkingice]]

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

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

Reviewed another patch from MozReview

Comment 21

[Carsten Book [:Tomcat]]

seems sebastian need to give final r+ here for autolander

Comment 22

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8823547 [details]
Fix for bug 1326374

https://reviewboard.mozilla.org/r/102082/#review111498

Comment 23

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8823547 [details]
Fix for bug 1326374

https://reviewboard.mozilla.org/r/102082/#review111608

The patch looks good but can you update the commit message to mention what your patch does?

Here are some helpful guides on how to write a good commit message:
* http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html#write-detailed-commit-messages
* https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code/#write-sensible-commit-me

Thanks!

Comment 24

[Sivaranjani Subramanian]

Yes, will make the necessary changes in the commit message.

Comment 25

[suman gupta]

Attachment: Bug 1326374 Changed the method spelling from Lollopop to lollipop.

Review commit: https://reviewboard.mozilla.org/r/115588/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/115588/

Comment 26

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8841333 [details]
Bug 1326374 Changed the method spelling from Lollopop to lollipop.

https://reviewboard.mozilla.org/r/115588/#review117768

Comment 27

[Julian Chu [:walkingice]]

Comment on attachment 8841333 [details]
Bug 1326374 Changed the method spelling from Lollopop to lollipop.

https://reviewboard.mozilla.org/r/115588/#review119462

Comment 28

[Julian Chu [:walkingice]]

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.

Comment 29

[suman gupta]

(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.

Comment 30

[suman gupta]

(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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/341ec10ef4c9
Changed the method spelling from Lollopop to lollipop. r=sebastian,walkingice

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/341ec10ef4c9