Closed
Bug 1109233
Opened 9 years ago
Closed 9 years ago
Replace `Assert.isTrue(false...)` with `Assert.fail(...)`
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 37
People
(Reporter: mcomella, Assigned: nivvedan, Mentored)
Details
(Whiteboard: [lang=java][good first bug])
Attachments
(1 file)
2.39 KB,
patch
|
mcomella
:
review+
|
Details | Diff | Splinter Review |
There are a few assertions in the code base that use `Assert.isTrue(false...)` [1]. For readability and simplicity of code, these are equivalent to, and should be replaced with, `Assert.fail(...)` [2]. To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android 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! ^_^ [1]: https://mxr.mozilla.org/mozilla-central/search?string=Assert.isTrue%28false&find=\.java%24&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central [2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Assert.java?rev=31f9bba23ae0#81
Reporter | ||
Updated•9 years ago
|
Mentor: liuche
Hi Micheal, I have just set up my build environment, and would like to work on this bug. Could you please assign me to it?
Comment 2•9 years ago
|
||
> I have just set up my build environment, and would like to work on this bug.
> Could you please assign me to it?
Just go ahead and start work; no need to be marked as assigned until we have a patch to show.
Reporter | ||
Comment 3•9 years ago
|
||
Sure, you've been assigned - let me know if you need anything!
Assignee: nobody → nivvedan
Status: NEW → ASSIGNED
Attachment #8534796 -
Flags: review?(michael.l.comella)
Reporter | ||
Comment 5•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=f6a826b4be89
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8534796 [details] [diff] [review] v1 Review of attachment 8534796 [details] [diff] [review]: ----------------------------------------------------------------- Great, looks good to me! In comment 5 I posted a link to a run of our Try server. When it goes green, feel free to add the "checkin-needed" keyword [1] to get your patch checked in. Note that all patches landed with checkin-needed need an associated green Try run. Of course, let me know if you need help reading the results. Thanks for your help! :) If you're looking for a good followup bug, I think bug 938845 would be straight-forward (but perhaps tedious D: !), while bug 926234 requires a bit of SQL knowledge to help untangle our database and bug 1108084 requires digging into our favicon code. If none of these work for you, let me know! [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Attachment #8534796 -
Flags: review?(michael.l.comella) → review+
Keywords: checkin-needed
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d95d0b4d113d
Flags: in-testsuite-
Keywords: checkin-needed
Whiteboard: [lang=java][good first bug] → [lang=java][good first bug][fixed-in-fx-team]
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d95d0b4d113d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [lang=java][good first bug][fixed-in-fx-team] → [lang=java][good first bug]
Target Milestone: --- → Firefox 37
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•