Closed
Bug 1242385
Opened 9 years ago
Closed 8 years ago
DownloadContentCatalog: markAsIgnored() is unused
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox49 fixed)
RESOLVED
FIXED
Firefox 49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: sebastian, Assigned: k.krish, Mentored)
References
Details
(Whiteboard: [lang=java][good first bug])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
sebastian
:
review+
|
Details | Diff | Splinter Review |
DownloadContentCatalog: markAsIgnored() is unused:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/dlc/catalog/DownloadContentCatalog.java#121-124
Remove the method, the constant and the test case covering this method.
To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android
Then, you'll need to create a patch to upload - see
https://wiki.mozilla.org/Mobile/Fennec/Android#Creating_commits_and_submitting_patches
If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "sebastian" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC
Reporter | ||
Updated•9 years ago
|
Whiteboard: [lang=java][good first bug]
Comment 1•9 years ago
|
||
Hi Sebastian, i am a noob looking to contribute to this project. Do you mind, if i assign myself to this ticket ?
Reporter | ||
Comment 2•9 years ago
|
||
(In reply to aerozeppelin from comment #1)
> Hi Sebastian, i am a noob looking to contribute to this project. Do you
> mind, if i assign myself to this ticket ?
Yeah, you can work on this. We usually assign the ticket after a patch has been uploaded.
comment 0 has some hints on how to setup a build environment. Let me know if you need any more helb.
Comment 3•9 years ago
|
||
Thanks Sebastian :D Here is my first attempt. Just wondering, what test should i run to check for regressions, robocop/xpcshell-test/mochitest ?
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → ajay051
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to aerozeppelin from comment #3)
> Thanks Sebastian :D Here is my first attempt. Just wondering, what test
> should i run to check for regressions, robocop/xpcshell-test/mochitest ?
Good question! In this case there are just some basic juni4 tests.
I assume you are using Android Studio:
* Open "Build variants" on the left side of the screen
* Select "Test artifact: Unit tests" (Not "Android Instrumentation tests").
* Search test package org.mozilla.gecko.dlc (Green background in Android Studio)
* Right click on "dlc" and select "Run 'All Tests'"
* Android Studio should run those tests locally without needing a device
* All tests should pass :)
Let me know if you need more help!
Comment 5•9 years ago
|
||
Thank you for outlining the steps. This is really helpful. :D
Reporter | ||
Comment 6•8 years ago
|
||
Oh, this has been lying around and we should land this!
Reporter | ||
Comment 7•8 years ago
|
||
Comment on attachment 8722363 [details] [diff] [review]
Patch for bug 1242385.
Unfortunately this patch does not apply anymore. :(
Attachment #8722363 -
Attachment is obsolete: true
Reporter | ||
Comment 8•8 years ago
|
||
Krish: This is another simple bug in the DLC area. Can you create a new patch for this? :)
Assignee: ajay051 → k.krish
Sebastian: Sure I will create a patch for this. I will make sure this patch contains Headers :)
Assignee | ||
Comment 10•8 years ago
|
||
Removed the method, constants and test method.
Attachment #8751718 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 11•8 years ago
|
||
Comment on attachment 8751718 [details] [diff] [review]
Bug1242385
Review of attachment 8751718 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM. Thanks!
Now that you have commit access level 1: Can you push the patch to try? :)
Here's some help:
https://wiki.mozilla.org/ReleaseEngineering/TryServer
If you have installed the mercurial extensions via |mach mercurial-setup| then it's just as easy as using |hg push-to-try -m 'try syntax'|
For the try syntax you can use: 'try: -b do -p android-api-15,android-x86,android-test,android-lint -u robocop -t none'
This should run everything that's important for most Android code.
You can use mcomella's helper to use try without remembering the syntax: https://github.com/mcomella/push-to-try-android
Or you can use the trychooser to build your own syntax: http://trychooser.pub.build.mozilla.org/
Let me know if I can help you. If all tasks are green on try then just add the 'checkin-needed' keyword to the bug and someone will land the patch for you! :)
Also look into using reviewboard. You can start try runs just from inside reviewboard and it's also a nicer tool for reviewers.. :)
Attachment #8751718 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Updated•8 years ago
|
Mentor: s.kaspari
Flags: needinfo?(k.krish)
Assignee | ||
Comment 12•8 years ago
|
||
Try Results
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4819ccaa38365930745655b0de9ecf949a75c282
checkin-needed
Flags: needinfo?(k.krish)
Keywords: checkin-needed
Comment 13•8 years ago
|
||
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder landing |
Comment 15•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Updated•4 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
•