Closed Bug 1188984 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox for Android :: General, defect)

42 Branch
Unspecified
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox42 --- affected
firefox43 --- fixed

People

(Reporter: sebastian, Assigned: gioyik, Mentored)

Details

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

Attachments

(1 file)

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
Summary: Remove 'public' and 'static' keywords from TabsPanel interface → Remove 'public' and 'static' keywords from nested interfaces and enums in TabPanel
I'd love to work on this bug! I've assigned it to myself.
Assignee: nobody → ma.steiman
Status: NEW → ASSIGNED
Never mind, I'm having serious build issues with android :\
Assignee: ma.steiman → nobody
Status: ASSIGNED → NEW
I'd like to work on this.
@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.
Yeah, i want to work on this.
(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. :)
Attached patch 1188984.patchSplinter Review
Attachment #8644804 - Flags: review?(s.kaspari)
Hi Sebastian,

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

Thanks
Will look at it today but it seems to be good. :)
Assignee: nobody → gioyik
Status: NEW → ASSIGNED
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?
Attachment #8644804 - Flags: review?(s.kaspari) → review+
I am familiar with try server and landing procedure :) Could you help me with the try syntax for this patch?

Thanks,
Gio
Flags: needinfo?(s.kaspari)
(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
Flags: needinfo?(s.kaspari)
Assignee: gioyik → gioyik
Sebastian, this is the patch on try server:

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

Hope it finish without problems. :)
Try server shows that everything is green!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/730d6db52063
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.