1188984 - Remove 'public' and 'static' keywords from nested interfaces and enums in TabPanel

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

A bunch of 'public' and 'static' modifiers at the top of TabsPanel are redundant. Let's remove them!

* Nested enum types are implicitly static[1]:
 * TabsPanel.Panel 

* Nested interfaces are always static:
 * TabsPanel.PanelView
 * TabsPanel.CloseAllPanelView
 * TabsPanel.TabsLayout
 * TabsPanel.TabsLayoutChangeListener

* Every method declaration in the body of an interface is implicitly public[2]
 * TabsPanel.PanelView.*()
 * TabsPanel.CloseAllPanelView.*()
 * TabsPanel.TabsLayout.*()
 * TabsPanel.TabsLayoutChangeListener.*()

To start, set up a build environment - you can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android

To fix this bug, you need to remove the redundant modifiers from this class:
https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tabs/TabsPanel.java

Then, you'll need to create a patch to upload - see
https://wiki.mozilla.org/Mobile/Fennec/Android#Creating_commits_and_submitting_patches

[1]: https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.9
[2]: https://docs.oracle.com/javase/specs/jls/se7/html/jls-9.html#jls-9.4

Comment 1

[Tyler Steiman]

I'd love to work on this bug! I've assigned it to myself.

Comment 2

[Tyler Steiman]

Never mind, I'm having serious build issues with android :\

Comment 3

[Divya]

I'd like to work on this.

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

@Muhsin: Let us now if we can help you fixing these build issues on IRC. My nick is "sebastian" and you can find me and other helpful people in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

@Divya: I see you grabbed bug 1189301 in the meantime. Let me know if you still want to work on this.

Comment 5

[Divya]

Yeah, i want to work on this.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Divya from comment #5)
> Yeah, i want to work on this.

Great! Do you have a build environment setup to build Fennec? You can see the instructions here: https://wiki.mozilla.org/Mobile/Fennec/Android

If you need any help with that then you can find me on IRC in #mobile.

After uploading a patch I'll assign the bug to you. :)

Comment 7

[Giovanny Gongora [:gioyik]]

Attachment: 1188984.patch

Comment 8

[Giovanny Gongora [:gioyik]]

Hi Sebastian,

I attached a patch for this bug. Could you check it and tell me if is OK?

Thanks

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Will look at it today but it seems to be good. :)

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8644804 [details] [diff] [review]
1188984.patch

Review of attachment 8644804 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect! Thank you, Giovanny.

Just add r=sebastian to your commit message and then you are ready to land. It seems like you landed already a bunch of patches, so I guess you are familiar with try and landing?

Comment 11

[Giovanny Gongora [:gioyik]]

I am familiar with try server and landing procedure :) Could you help me with the try syntax for this patch?

Thanks,
Gio

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Giovanny Gongora [:gioyik] from comment #11)
> I am familiar with try server and landing procedure :) Could you help me
> with the try syntax for this patch?

Sure! I prefer to use TryChooser: http://trychooser.pub.build.mozilla.org/ and select:

* Build types: Both
* Platforms: All Android
* Test suites: All robocop

try: -b do -p android-api-9,android-api-11,android-x86 -u robocop-1,robocop-2,robocop-3,robocop-4,robocop-5,robocop-6,robocop-7,robocop-8,robocop-9,robocop-10 -t none

But in theory it should be enough to just specify -u robocop:

try: -b do -p android-api-9,android-api-11,android-x86 -u robocop -t none

Comment 13

[Giovanny Gongora [:gioyik]]

Sebastian, this is the patch on try server:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=26e284ea1109

Hope it finish without problems. :)

Comment 14

[Giovanny Gongora [:gioyik]]

Try server shows that everything is green!

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/730d6db52063

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/730d6db52063