Closed Bug 1261210 Opened 10 years ago Closed 10 years ago

Rename omg.mozglue.ContextUtils -> omg.mozglue.SafeIntentUtils

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox49 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 49
Tracking Flags:
Tracking Status
firefox49 --- fixed

People

(Reporter: mcomella, Assigned: jpfz03, Mentored)

References

Details

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

Attachments

(1 file, 5 obsolete files)

New Patch for Bug 1261210
12.36 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
To start, set up a build environment - you can see the instructions here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build Then, you'll need to create a patch to upload - see https://wiki.mozilla.org/Mobile/Fennec/Android/Suggested_workflow#Creating_commits_and_submitting_patches You can find more miscellaneous information at: https://wiki.mozilla.org/Mobile/Fennec/Android To fix this bug, use an IDE to make the rename and test the app to make sure nothing out of the ordinary is happening! If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC Thanks and happy coding! ^_^
Hi Michael, I want to work on this bug. Just wondering is the renaming happening at the front-end part or the back-end part?
(In reply to Ryan from comment #1) > Hi Michael, I want to work on this bug. Just wondering is the renaming > happening at the front-end part or the back-end part? Hey Ryan – this is a "front-end" bug – as in, it does not involve the Gecko platform code.
Also, welcome! :) We'll assign you once you post a patch.
Hey Michael, I am looking to work on the bug but not sure on the testing required? Will successful build + tests suffice for this bug?
(In reply to Akshat Goel from comment #4) > I am looking to work on the bug but not sure on the testing required? Will > successful build + tests suffice for this bug? Hi Akshat. A build, the tests (which we can run on the test servers), and testing locally (e.g. load a few pages) should suffice. Ryan, are you still working on this bug? Akshat, if Ryan doesn't respond within a few days, you can take it, otherwise it's probably better to find another [good first bug].
Flags: needinfo?(leung.ryan.ca)
Hi Michael, Yes I am planning to work on this bug. It took me a while to compile the code. Thanks, Ryan
Flags: needinfo?(leung.ryan.ca)
Hi Michael, Ok, I plan to work on this bug.
Hey Michael, Is this bug still available? I'm planning to submit my committed changes over through MozReview to the reviewboard although I didn't specify my reviewers. However, if someone already took the bug, I'll work on something else. Thanks!
Hi Kevin – I believe Juan was already working on this so I'd prefer to take Juan's patch if possible. Juan, did you start on this bug yet? If so, do you think you'll have a patch soon? If not, do you mind if I find you another one? I apologize for the inconvenience.
Flags: needinfo?(jpfz03)
Hi, Michael Yes I finished compiling the code and I think I might give a patch soon.
Attached patch patchfile.patch (obsolete) — — Splinter Review
Attached patch Patch For Bug 1261210 (obsolete) — — Splinter Review
Attachment #8749449 - Attachment is obsolete: true
Attachment #8749455 - Flags: review?(michael.l.comella)
Comment on attachment 8749455 [details] [diff] [review] Patch For Bug 1261210 Review of attachment 8749455 [details] [diff] [review]: ----------------------------------------------------------------- This looks good except the SafeUtils file is missing! Did you forget to add it? You should really use the `hg mv <src> <dst>` command to rename files though.
Attachment #8749455 - Flags: review?(michael.l.comella) → feedback+
Assignee: nobody → jpfz03
Hi Michael, Sorry for this omission, and I hope that the new patch is good.
Attachment #8749455 - Attachment is obsolete: true
Attachment #8749973 - Flags: review?(michael.l.comella)
Comment on attachment 8749973 [details] [diff] [review] Patch For Bug Bug 1261210 with renamed files Review of attachment 8749973 [details] [diff] [review]: ----------------------------------------------------------------- The changes in this patch look good but unfortunately, it appears to be in the wrong format and I can't apply it. Either check that it's in the correct format [1] or use our more robust mozreview system. [2] [1]: https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F [2]: https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview-user.html
Attachment #8749973 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8749973 [details] [diff] [review] Patch For Bug Bug 1261210 with renamed files I meant to set no r+ because I can't check this in if it doesn't apply.
Attachment #8749973 - Flags: review+ → feedback+
Attached patch Patch for Bug 1261210 (obsolete) — — Splinter Review
Attachment #8749973 - Attachment is obsolete: true
Attachment #8750916 - Flags: review?(michael.l.comella)
Hi Michael, Ok I understand and I changed the format with hg diff. (Hoping you can apply it).
Comment on attachment 8750916 [details] [diff] [review] Patch for Bug 1261210 Hey Juan. (In reply to Juan Fernandez from comment #20) > Ok I understand and I changed the format with hg diff. > (Hoping you can apply it). The patch is in the correct format (I could apply it, yay!) but unfortunately, there were merge conflicts because your patch seems to be missing our latest changes. Can you pull down the latest changes and rebase your patch on top? If there are still issues after that (e.g. since the code underneath is constantly changing), I can do the rebase on my end and land it so we can get you on your way to your next bug! :)
Attachment #8750916 - Flags: review?(michael.l.comella) → feedback+
Attachment #8750916 - Attachment is obsolete: true
Attachment #8751998 - Flags: review?(michael.l.comella)
Hi Michael, I pull down the latest changes and I create the patch again.
Awesome, it looks good, Juan! Thanks for hanging in there! https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c6e2340d42f I made a push to our try test servers (above). Once the push goes green, you can add the "checkin-needed" keyword [1] to get your patch checked in. Note that all patches added via checkin-needed keyword need an associated green try run. Let me know if you need help reading the results. [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Attachment #8751998 - Flags: review?(michael.l.comella) → review+
Hi Michael, I've just reviewed the job and it seems all the builds have succeeded except that a couple tests did fail. Looking at the exceptions, they don't seem to be related to this bug. Can I add "checkin-needed" anyway?
(In reply to Juan Fernandez from comment #25) > I've just reviewed the job and it seems all the builds have succeeded except > that a couple tests did fail. Looking at the exceptions, they don't seem to > be related to this bug. Can I add "checkin-needed" anyway? We sometimes have intermittent issue and this looks like some of them – you should be good to add "checkin-needed"! Thanks for your help! By the way, you can use the "Need more information from" field to give a user a persistent notification so they'll be more likely to see your comments or questions.
(In reply to Michael Comella (:mcomella) from comment #26) > We sometimes have intermittent issue and this looks like some of them – you > should be good to add "checkin-needed"! Thanks for your help! Actually, some work I'm doing needs this so I'm just going to land it – no need to add the keyword. Thanks again!
Flags: needinfo?(jpfz03)
Keywords: checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aa7ea6cc0792
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: