1122329 - Remove `= null` from `instance = null` in StringHelper

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We initialize our singleton instance in mobile/android/base/tests/StringHelper.java to null, however, Java does this for us automatically. Remove the initialization and keep the declaration.

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

The tests you would be fixing are in our Robocop test framework, so you'll need to set that up too: https://wiki.mozilla.org/Auto-tools/Projects/Robocop

When you make changes in the mobile/android/base/tests/ directory, you should only need to recompile Robocop, not the entire browser, for these changes.

While you're developing, I recommend running just a single test at a time because the full robocop test suite takes a long time to run (at least 30 minutes).

If you need any help, you can reply to this bug, or feel free to message me on IRC - my nick is "mcomella" and you can find me in #mobile. If you need IRC setup instructions, see https://wiki.mozilla.org/IRC

Thanks and happy coding! ^_^

Comment 1

[AMIT JAIN]

sir,
i am a newbie and i want start contributing in mozilla.I think this could be my first bug.
So could you please little bit explain how to do it.

Comment 2

[Aaron Train [:aaronmt]]

Comment #0 has precise instructions

Comment 3

[AMIT JAIN]

i want ask that whether i want to download any other mozilla mobile source code for mobile because my source code(which is for desktop i think) contain that folder which was described in context

and 

i was unable to find any null in that file(mozilla-central/mobile/android/base/tests/StringHelper.java)
this bug is still there or not OR i am doing something wrong

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, Martyn. It seems you may have assigned this bug to the wrong person - is there something I missed?

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hi, Amit.

We'll get the assignee business sorted out shortly. In the mean time...

I'm sorry for the confusion! This bug is dependent on bug 938845, which is pretty close to landing. If you want to get a headstart (and don't mind doing a little extra bookkeeping work), apply the instructions from bug 1122331 comment 2.

(In reply to AMIT JAIN from comment #3)
> i was unable to find any null in that
> file(mozilla-central/mobile/android/base/tests/StringHelper.java)
> this bug is still there or not OR i am doing something wrong

bug 938845, which hasn't landed yet, actually has the line in question.

> i want ask that whether i want to download any other mozilla mobile source
> code for mobile because my source code(which is for desktop i think) contain
> that folder which was described in context

I'm not sure I understand your question - do you mind rephrasing?

Comment 6

[Anish]

why would I be the wrong person Michael Comella

Comment 7

[Martyn Haigh (:mhaigh)]

Apologies for the mix up - Anish was looking at this on the 16th but we were busy getting his build up and working and didn't assign the bug.  When I did assign, I failed to notice that more comments had been added despite the bug still remaining unclaimed. 

Guys, does anyone fancy volunteering to give this bug up and look at another?

Comment 8

[Anish]

Ok ! let him have it :) all the best

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Thanks Anish! Sorry for the mix-up!

Comment 10

[Bhargav Chippada [:bhargavch]]

If no one is working on this bug, can i be assigned to it?

Comment 11

[Martyn Haigh (:mhaigh)]

There's been no update on this bug for over a month, so I'm happy for you to take it.

Comment 12

[Bhargav Chippada [:bhargavch]]

Hi, this bug seems to not exist. I can't find instance = null inside StringHelper. This bug depends on bug 938845 which hasn't landed yet right? May be i will skip this bug and do some other one or am i missing anything?

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Bhargav Chippada [:bhargavch] from comment #12)
> This bug depends on bug 938845 which hasn't landed yet right?
> May be i will skip this bug and do some other one or am i missing anything?

That's correct - sorry about that!

Let me know if you need help finding bugs.

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Unassigning due to inactivity.

Comment 15

[Melchiorre Alastra (:Mailkov)]

I'd like to work on this ... can i be assigned to it?

Comment 16

[Melchiorre Alastra (:Mailkov)]

Attachment: bug1122329.patch

Comment 17

[Martyn Haigh (:mhaigh)]

Comment on attachment 8631192 [details] [diff] [review]
bug1122329.patch

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

Hey Melchiorre, great job on your first patch.  You missed out specifying a reviewer when you submitted your patch - you need to select the "?" next to the "Review" label under flags on the "Create new attachment" page and then enter a user name in the text field next to it.  If you don't know who should review it, click the "Suggested reviewers" link and that will suggest some people for you.

For the moment I'm going to approve your review (we call this a r+) which means that you're good to land the patch.  It's good practice to push your patch to our Try server [1] first, to make sure you don't break anything.

Well done :)


1, https://wiki.mozilla.org/ReleaseEngineering/TryServer

Comment 18

[Melchiorre Alastra (:Mailkov)]

Please, can someone vouch for me ?  https://mozillians.org/en-US/u/Mailkov/

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

NI self: push to try!

Comment 20

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=17a1c93ee21f

Comment 21

[Melchiorre Alastra (:Mailkov)]

I think that the try on treeherder is ok ... what is the next step?

If you vouch for me ... I'll use treeherder for the next bug alone.

Comment 22

[Martyn Haigh (:mhaigh)]

Mailkov: Once you have a r+ on your patch, it's time to push.  As you don't have commit access at the moment you'll need to add the the checkin-needed keyword to the bug, which means that a sheriff will come and commit your reviewed patch.  I've done this for you for this bug.  It'd be good to get another couple of patches under your belt before you get commit access, just so you have a chance to learn the ropes a little more.

You shouldn't need to do anything on this bug now - time to look for another bug you fancy tackling!

Comment 23

[Pulsebot]

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

Comment 24

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

https://hg.mozilla.org/mozilla-central/rev/adfa7cdbf5e6

Comment 25

[Melchiorre Alastra (:Mailkov)]

Ok, thanks