1242385 - DownloadContentCatalog: markAsIgnored() is unused

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

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

Comment 1

[aerozeppelin]

Hi Sebastian, i am a noob looking to contribute to this project. Do you mind, if i assign myself to this ticket ?

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

(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

[aerozeppelin]

Attachment: Patch for bug 1242385.

Thanks Sebastian :D Here is my first attempt. Just wondering, what test should i run to check for regressions, robocop/xpcshell-test/mochitest ?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

(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

[aerozeppelin]

Thank you for outlining the steps. This is really helpful. :D

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Oh, this has been lying around and we should land this!

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8722363 [details] [diff] [review]
Patch for bug 1242385.

Unfortunately this patch does not apply anymore. :(

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Krish: This is another simple bug in the DLC area. Can you create a new patch for this? :)

Comment 9

[k.krish]

Sebastian: Sure I will create a patch for this. I will make sure this patch contains Headers :)

Comment 10

[k.krish]

Attachment: Bug1242385

Removed the method, constants and test method.

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

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.. :)

Comment 12

[k.krish]

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

checkin-needed

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/731fde2902c9

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/fx-team/rev/731fde2902c9

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/731fde2902c9