Closed
Bug 1259669
Opened 8 years ago
Closed 8 years ago
Clean up WidgetCommandEvent
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: masayuki, Assigned: hogehoge, Mentored)
References
Details
(Whiteboard: [good first bug])
Attachments
(2 files)
3.13 KB,
patch
|
Details | Diff | Splinter Review | |
58 bytes,
text/x-review-board-request
|
masayuki
:
review+
|
Details |
See bug 1259654 comment 0 for the detail.
Assignee | ||
Comment 1•8 years ago
|
||
Hi, can I take this? I'm new and would like to try.
Reporter | ||
Comment 2•8 years ago
|
||
(In reply to Takahiro Aoyagi from comment #1) > Hi, can I take this? > I'm new and would like to try. Of course. Do I need to explain how to fix this?
Flags: needinfo?(hogehoge)
Assignee | ||
Comment 3•8 years ago
|
||
Just in case, could you explain tho I think I basically understand from bug 1259654?
Flags: needinfo?(hogehoge)
Reporter | ||
Comment 4•8 years ago
|
||
Okay, so, this is one of the simplest fix of blocking bug 1259654. WidgetCommandEvent is defined in widget/MiscEvents.h and its member |command| doesn't conform to our coding rules. In our coding rules, all members should have "m" prefix. So, |command| should be renamed to |mCommand|. https://dxr.mozilla.org/mozilla-central/source/widget/MiscEvents.h#104,113,124,130 I guess that if you build Firefox after writing a patch, it must be correct fix. But I'll post your patch to tryserver to test to build in whole platforms we support. Do you know how to write patch and request review? If not, please read following documents: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style http://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/index.html Especially here: http://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/firefoxworkflow.html If you have some questions, feel free to ask me. (I should write simpler tutorial in my blog, though...)
Assignee | ||
Comment 5•8 years ago
|
||
Hi, I've made a patch and succeeded in build with this change. I'm not confident about how to make a patch. The following commands are what I did. Is this a correct flow? Or is there any better ways? $ hg clone $ change code $ hg commit -m "comment" $ hg export -r changeset -o bug1259669.patch
Attachment #8742156 -
Flags: review?(masayuki)
Reporter | ||
Comment 6•8 years ago
|
||
First, thank you for your work! (In reply to Takahiro Aoyagi (github@takahirox) from comment #5) > Created attachment 8742156 [details] [diff] [review] > bug1259669.patch > > Hi, I've made a patch and succeeded in build with this change. > > I'm not confident about how to make a patch. > The following commands are what I did. > Is this a correct flow? Or is there any better ways? > > $ hg clone Did you do ./mach mercurial-setup? Then, you can set up whole special utilities for developing Gecko/Firefox. > $ change code > $ hg commit -m "comment" Exactly. > $ hg export -r changeset -o bug1259669.patch If you did the setup and set up SSH connection which is documented here: http://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/auth.html#ssh-configuration Then, you must be able to do this: $ hg push -c changeset review or $ hg push -r firstchangeset:lastchangeset review Then, your patch will be sent to ReviewBoard automatically. After that I can build your patch on all platforms on tryserver from ReviewBoard. Could you do that? If you couldn't set up ReviewBoard and SSH related settings, I'd try to post your attached patch to tryserver by my hand.
Assignee | ||
Comment 7•8 years ago
|
||
Yeah, I ran ./mach mercurial-setup with all Y to wizard. But I've failed to run "hg push -c changeset review" due to permission denied(publickey). I login to bugzilla with GitHub account but should I get a bugzilla account to push to review? $ cat ~/.ssh/config Host hg.mozilla.org User hogehoge@gachapin.jp Host reviewboard-hg.mozilla.org User hogehoge@gachapin.jp $ hg push -c 293163:fec01c615b5b review pushing to ssh://reviewboard-hg.mozilla.org/gecko remote: Permission denied (publickey). abort: no suitable response from remote hg!
Reporter | ||
Comment 8•8 years ago
|
||
Ikezoe-san, do you know how to fix the issue mentioned in comment 7?
Flags: needinfo?(hiikezoe)
Comment 9•8 years ago
|
||
(In reply to Takahiro Aoyagi (github@takahirox) from comment #7) > Yeah, I ran ./mach mercurial-setup with all Y to wizard. > > But I've failed to run "hg push -c changeset review" due to permission > denied(publickey). > I login to bugzilla with GitHub account but should I get a bugzilla account > to push to review? > > $ cat ~/.ssh/config > Host hg.mozilla.org > User hogehoge@gachapin.jp > > Host reviewboard-hg.mozilla.org > User hogehoge@gachapin.jp > > $ hg push -c 293163:fec01c615b5b review > pushing to ssh://reviewboard-hg.mozilla.org/gecko The problem is here. This should be https://reviewboard-hg.mozilla.org/autoreview . You need below line in your ~/.hgrc or .hg/hgrc in source tree. review = https://reviewboard-hg.mozilla.org/autoreview
Flags: needinfo?(hiikezoe)
Assignee | ||
Comment 10•8 years ago
|
||
Thanks guys, I should've read this page http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/install-mercurial.html Anyways, I seem like succeeded in push to review. https://reviewboard.mozilla.org/r/47413/
Comment 11•8 years ago
|
||
Good to hear. Not published the review yet. ;-)
Assignee | ||
Comment 12•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47415/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47415/
Reporter | ||
Comment 13•8 years ago
|
||
Thank you very much. I posted the patch to tryserver: https://treeherder.mozilla.org/#/jobs?repo=try&revision=213f88ef61e0
Reporter | ||
Comment 14•8 years ago
|
||
Comment on attachment 8742683 [details] MozReview Request: Bug 1259669 Rename WidgetCommandEvent::command to WidgetCommandEvent::mCommand r=masayuki https://reviewboard.mozilla.org/r/47415/#review44125 Okay, all green. Could you change the commit message to: "Bug 1259669 Rename WidgetCommandEvent::command to WidgetCommandEvent::mCommand r=masayuki" for consistency with other commits (you should write the detail of what you did in the changeset) and push it to review again? Then, I'll land the patch to mozilla-inbound. Note that you can change the commit message with |hg histedit| and modify "pick" to "m" of the text file opened by histedit and save and quit the editor. Then, you can modify the commit message. Thanks in advance!
Attachment #8742683 -
Flags: review+
Assignee | ||
Comment 15•8 years ago
|
||
Comment on attachment 8742683 [details] MozReview Request: Bug 1259669 Rename WidgetCommandEvent::command to WidgetCommandEvent::mCommand r=masayuki Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47415/diff/1-2/
Attachment #8742683 -
Attachment description: MozReview Request: Bug 1259669 - Clean up WidgetCommandEvent → MozReview Request: Bug 1259669 Rename WidgetCommandEvent::command to WidgetCommandEvent::mCommand r=masayuki
Assignee | ||
Comment 16•8 years ago
|
||
My commit log update went on correctly?
Reporter | ||
Comment 17•8 years ago
|
||
Oh, sorry, I missed to catch your patch failed to autoland into mozilla-inbound. I'll try to land it manually.
Reporter | ||
Comment 18•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7aeac9da4265727deef801de5416f1f50c9a74dd Bug 1259669 Rename WidgetCommandEvent::command to WidgetCommandEvent::mCommand r=masayuki
Assignee | ||
Comment 19•8 years ago
|
||
Thanks > author Takahiro Aoyagi <github@takahirox> If possible, could you replace my mail address with hogehoge@gachapin.jp?
Comment 20•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7aeac9da4265
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Reporter | ||
Comment 21•8 years ago
|
||
(In reply to Takahiro Aoyagi (takahirox@github) from comment #19) > Thanks > > > author Takahiro Aoyagi <github@takahirox> > > If possible, could you replace my mail address with hogehoge@gachapin.jp? Oh, I'm very sorry. I copied it from your bugzilla account name at committing your patch into my local repository manually (due to conflicting with other commits). The patch was landed to Mozilla 49, fortunately. So, I can reland it with the email address again. However, even if I'll do that, the old commit log isn't removed (modified), unfortunately. But if you want me to do that, I'll do it.
Flags: needinfo?(hogehoge)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → hogehoge
Whiteboard: [good first bug][for Gecko Inside #7 in Mozilla Japan] → [good first bug]
Reporter | ||
Updated•8 years ago
|
Attachment #8742156 -
Flags: review?(masayuki)
Assignee | ||
Comment 22•8 years ago
|
||
So, could you do that for me if it wouldn't be too much trouble for you.
> However, even if I'll do that, the old commit log isn't removed (modified), unfortunately.
Yeah, that't ok.
Reporter | ||
Comment 23•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d1d2245fa0092fe49d4e6287d4417e3e051a151 Backout bug 1259669 due to commited with wrong user name https://hg.mozilla.org/integration/mozilla-inbound/rev/95d06a6cc1acaff9ac47d9a071844f8f9e9deacd Bug 1259669 Rename WidgetCommandEvent::command to WidgetCommandEvent::mCommand r=masayuki
Reporter | ||
Comment 24•8 years ago
|
||
(In reply to Takahiro Aoyagi (takahirox@github) from comment #22) > So, could you do that for me if it wouldn't be too much trouble for you. > > > However, even if I'll do that, the old commit log isn't removed (modified), unfortunately. > > Yeah, that't ok. Sorry for the delay, I relanded your patch with right email address right now.
Flags: needinfo?(hogehoge)
Comment 25•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/95d06a6cc1ac
Assignee | ||
Comment 26•8 years ago
|
||
Thanks! I had a great experience. I'll try to keep contributing to Mozilla!
You need to log in
before you can comment on or make changes to this bug.
Description
•