DownloadContentCatalog: markAsIgnored() is unused

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: sebastian, Assigned: k.krish, Mentored)

Tracking

unspecified
Firefox 49
All
Android
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

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
Whiteboard: [lang=java][good first bug]

Comment 1

3 years ago
Hi Sebastian, i am a noob looking to contribute to this project. Do you mind, if i assign myself to this ticket ?
(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

3 years ago
Posted patch Patch for bug 1242385. (obsolete) — Splinter Review
Thanks Sebastian :D Here is my first attempt. Just wondering, what test should i run to check for regressions, robocop/xpcshell-test/mochitest ?
Assignee: nobody → ajay051
Status: NEW → ASSIGNED
(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

3 years ago
Thank you for outlining the steps. This is really helpful. :D
Oh, this has been lying around and we should land this!
Comment on attachment 8722363 [details] [diff] [review]
Patch for bug 1242385.

Unfortunately this patch does not apply anymore. :(
Attachment #8722363 - Attachment is obsolete: true
Krish: This is another simple bug in the DLC area. Can you create a new patch for this? :)
Assignee: ajay051 → k.krish
(Assignee)

Comment 9

3 years ago
Sebastian: Sure I will create a patch for this. I will make sure this patch contains Headers :)
(Assignee)

Comment 10

3 years ago
Posted patch Bug1242385Splinter Review
Removed the method, constants and test method.
Attachment #8751718 - Flags: review?(s.kaspari)
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+
Mentor: s.kaspari
Flags: needinfo?(k.krish)
(Assignee)

Comment 12

3 years ago
Try Results 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4819ccaa38365930745655b0de9ecf949a75c282

checkin-needed
Flags: needinfo?(k.krish)
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 15

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/731fde2902c9
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
You need to log in before you can comment on or make changes to this bug.