Closed Bug 1158979 Opened 5 years ago Closed 2 years ago

Remove StringHelper.get and replace the StringHelper assignment with StringHelper.initialize

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: mcomella, Assigned: sreeise, Mentored, NeedInfo)

References

Details

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

Attachments

(1 file, 10 obsolete files)

59 bytes, text/x-review-board-request
cnevinchen
: review+
Details
Once bug 1121651 lands, we no longer have to access the StringHelper singleton via the get [1] method so remove it.

Since we still need the StringHelper reference, have initialize [2] return the instance and make the assignment using that return value at [3]. Continue to ensure initialize won't be called twice.

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! ^_^

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/StringHelper.java?rev=973f507abcf0#403
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/StringHelper.java?rev=973f507abcf0#396

[3]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/BaseRobocopTest.java?rev=34167fac3d3a#149
Hi Michael,

this sounds like an interesting and good starting point. I would like to take care of this bugfix.
I will follow your instructions through and connect to the IRC channel so we can communicate.

Kind regards,
Martin
Hey, Martin. Unfortunately, bug 1121651 will need to land before you can work on this bug (or alternatively, you can take the patch in that bug and work on top of that, which will require some more complex use of version control if you're up for it). If you want to continue on this bug, then go for it, but otherwise, perhaps you'd like to take a look at bug 1122329?
Hi Michael, sure I can take a look at bug 1122329. Thank you
Hi Michael

I think I can fix this bug. I see that the dependency bug 1122329 has been resolved and fixed. Is it a good time to start work on this bug?
Hey Saraj – welcome to Bugzilla!

Now would be a good time to start working on the bug so go for it! We'll assign you once you post a patch.

Let us know if you have any questions!
Its been nearly half a month since Sara Posted on this, and not to take it away from her, I would like to begin work on this with my team as a 1st bug if that is ok? We are simply looking for experience in going through the process and the possibility of having a mentor.

Thanks!
(In reply to Nick from comment #6)
> Its been nearly half a month since Sara Posted on this, and not to take it
> away from her, I would like to begin work on this with my team as a 1st bug
> if that is ok? We are simply looking for experience in going through the
> process and the possibility of having a mentor.

Sure, Nick. Let us know if you have any questions.
By the way, you can use the "Need more information from" field to give a user a persistent notification so they'll be more likely to see your comments or questions.
(In reply to Michael Comella (:mcomella) from comment #8)
> By the way, you can use the "Need more information from" field to give a
> user a persistent notification so they'll be more likely to see your
> comments or questions.

Ok so I got the build up and going, and Robocop Installed when I build Robocop I get the Following:

Ran 2236 Test (75 Parents, 2161 subtests)
Expected results: 2195
Unexpected Results: 4 (FAIL: 4)
=======================================
testabouthomeVisibility
---------------------------------
FAIL the toolbar title is mochi.test:8888/tests/robocop/robocop_blank_02.html
FAIL Exception caught
testEventDispatcher
--------------------------------
FAIL should have completed event before timeout
FAIL Exception caught

So those look pretty minor? Is this normal or should I at least be good to go?

lastly am I looking for this method in the "mobile/android/base/tests/" directory or should i be looking elsewhere?

Thanks for all your help
Flags: needinfo?(michael.l.comella)
Hey Nick.

Our test suites often get intermittent failures and these look like they could be some of them. Typically we run our tests on our testing infrastructure (calling Try) so we can retrigger each failure a few times to verify if the failure is intermittent or not. You can simulate this locally by re-running these tests several times locally.

I'd generally recommend using try though – it takes a lot of time to run the test suites locally and the device configurations on try can be slightly different than when you run locally (e.g. we use emulators or try). If you post your WIP patch, I can push it to the test servers and once you fix a few bugs, you can get access to push to try yourself!

As for where to look, you can use https://mxr.mozilla.org/ to search through the code base for the code you're looking for. e.g., we're trying remove `StringHelper.get` so I might search for `class StringHelper` [1] or search for the `StringHelper.java` file. Note that you should look in the mozilla-central repository.

[1]: https://mxr.mozilla.org/mozilla-central/search?string=class%20stringhelper&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Flags: needinfo?(michael.l.comella)
Attached patch StringHelper.java WIP (obsolete) — Splinter Review
Could you please push to the test servers thank you :)
Attachment #8722193 - Flags: review+
Attachment #8722193 - Flags: feedback+
Attached patch WIP-BaseRobocopTest.java (obsolete) — Splinter Review
Please push to the test servers thank you :)
Attachment #8722195 - Flags: review+
Attachment #8722195 - Flags: feedback+
(In reply to Michael Comella (:mcomella) from comment #10)
> Hey Nick.
> 
> Our test suites often get intermittent failures and these look like they
> could be some of them. Typically we run our tests on our testing
> infrastructure (calling Try) so we can retrigger each failure a few times to
> verify if the failure is intermittent or not. You can simulate this locally
> by re-running these tests several times locally.
> 
> I'd generally recommend using try though – it takes a lot of time to run the
> test suites locally and the device configurations on try can be slightly
> different than when you run locally (e.g. we use emulators or try). If you
> post your WIP patch, I can push it to the test servers and once you fix a
> few bugs, you can get access to push to try yourself!
> 
> As for where to look, you can use https://mxr.mozilla.org/ to search through
> the code base for the code you're looking for. e.g., we're trying remove
> `StringHelper.get` so I might search for `class StringHelper` [1] or search
> for the `StringHelper.java` file. Note that you should look in the
> mozilla-central repository.
> 
> [1]:
> https://mxr.mozilla.org/mozilla-central/
> search?string=class%20stringhelper&find=mobile%2Fandroid&findi=&filter=^[^\0]
> *%24&hitlimit=&tree=mozilla-central

Thanks for your Help Michael, I have attached both the java classes we have patched. Could you please push those to the test server for me and see if we are good to go or further edits need to be made?

Thank you again!
Nick
Flags: needinfo?(michael.l.comella)
Hi Nick.

Instead of uploading the full Java source file, can you upload a patch file, as per https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Creating_a_patch ?
Assignee: nobody → phrozen4life
Flags: needinfo?(michael.l.comella) → needinfo?(phrozen4life)
Attached patch stringHelper.diff (obsolete) — Splinter Review
Ok hope this works a little better :)
Attachment #8722193 - Attachment is obsolete: true
Attachment #8722195 - Attachment is obsolete: true
Flags: needinfo?(phrozen4life) → needinfo?(michael.l.comella)
Comment on attachment 8723259 [details] [diff] [review]
stringHelper.diff

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

Make sure you compile and test your changes (see comment 0)! As the try push shows, this code doesn't compile.

::: build/mozconfig.common
@@ +18,5 @@
>  
>  . "$topsrcdir/build/mozconfig.automation"
> +
> +# Build Firefox for Android:
> +ac_add_options --enable-application=mobile/android

You shouldn't be editing the common mozconfig with this patch.

If you're not already doing so, you should be able to build without using the root mozconfigs – you can create your local mozconfig in <path-to-mozilla-central>/mozconfig.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/StringHelper.java
@@ +383,3 @@
>      }
>  
> +/**

You don't need to comment out the old method (and its usages) – just remove it! If we need it again, we can get it from version control.
Attachment #8723259 - Flags: feedback+
Flags: needinfo?(michael.l.comella)
Attached patch stringHelper.patch (obsolete) — Splinter Review
Ok not flagging anymore errors from the testBackButtonInEditMode, would you mind taking a look to ensure we found all files :)
Attachment #8723259 - Attachment is obsolete: true
Flags: needinfo?(michael.l.comella)
Comment on attachment 8727203 [details] [diff] [review]
stringHelper.patch

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

It looks like this patch builds upon your previous patch (e.g. initialize doesn't have a return value in my local build) – can you combine them?

btw, it's more correct to flag me for feedback or review on your patch than to use the needinfo flag.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java
@@ +174,5 @@
>          // Set up Robotium.solo and Driver objects
>          Activity tempActivity = getActivity();
>  
>          mStringHelper = StringHelper.initialize(tempActivity.getResources());
> +        

nit: We try not to add excess whitespace to files and there are spaces on this line – can you remove the spaces to make this an empty line?

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/GeckoClickHelper.java
@@ +34,5 @@
>      public static void openCentralizedLinkInNewTab() {
>          openLinkContextMenu();
>  
>          // Click on "Open Link in New Tab"
> +        sSolo.clickOnText(StringHelper.initialize().CONTEXT_MENU_ITEMS_IN_NORMAL_TAB[0]);

You shouldn't call `initialize` directly – that should throw because it's already been called (did you try running the tests?). You should be able to get the reference from BaseRobocopTest, probably through the UITestContext.
Attachment #8727203 - Flags: feedback+
Flags: needinfo?(michael.l.comella)
Unassigning due to inactivity.
Assignee: phrozen4life → nobody
Hello Michael, When I try to look at the links mentioned in the description, it says that the file does not exists. Could you help me get started?
Flags: needinfo?(michael.l.comella)
(In reply to Akshay Jain from comment #21)
> Hello Michael, When I try to look at the links mentioned in the description,
> it says that the file does not exists. Could you help me get started?

Hey Akshay. Our test files were moved:
  https://dxr.mozilla.org/mozilla-central/search?q=file%3AStringHelper.java&redirect=true&case=true
Flags: needinfo?(michael.l.comella)
Hi there,

I'd like to start working on this bug and I'm quite new to this. Do you have any advice on how to proceed?

Thanks
(In reply to ben94lim from comment #23)
> Hi there,
> 
> I'd like to start working on this bug and I'm quite new to this. Do you have
> any advice on how to proceed?
> 
> Thanks

Hi ben94lim, I am working on this and most probably I will submit the patch soon.
Can we get a sense of how this is coming along?
Flags: needinfo?(ajain2)
Please review this patch and let me know of any changes required. Thank You!
Attachment #8793385 - Flags: review?(michael.l.comella)
Comment on attachment 8793385 [details] [diff] [review]
Fixes Bug 1158979 --- Removed StringHelper.get and replaced the StringHelper assignment with StringHelper.initialize

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

Hey – thanks for the patch!

Unfortunately, I only see your changes to BaseRobocopTest. Perhaps some changes were left out of version control?

Let me know if you need any help with hg!
Attachment #8793385 - Flags: review?(michael.l.comella) → feedback+
Assignee: nobody → baishya.anuraag1920
Flags: needinfo?(ajain2)
Added Changes to all required files. (Hopefully)
Attachment #8794072 - Attachment is obsolete: true
Attachment #8794072 - Flags: review?(michael.l.comella)
Attachment #8794073 - Flags: review?(michael.l.comella)
 How do I work on this bug?
(In reply to mddrill from comment #30)
>  How do I work on this bug?

Hi mddrill. Unfortunately, this bug is currently being worked on – perhaps you'd like to take a look at bug 1118971?
Comment on attachment 8794073 [details] [diff] [review]
Removed unnecessary function StringHelper.get(), changed return type of StringHelper.initialize from void to StringHelper, carried out required changes in GeckoClickHelper and BaseRobocopTest

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

Sorry I wrote the description unclearly – the underlying code has changed since I wrote it!

 * `StringHelper.get` needs to be removed
 * `StringHelper.initialize` should be called only once to initialize the member variable in BaseRobocopTest. initialize should be written to throw if it is called more than once.
 * `StringHelper.get`'s callers should be replaced by accessing the BaseRobocopTest.mStringHelper instance. In the case of GeckoClickHelper, you should be able to find this as part of UITestContext.

Please review comment 0 for more details.

I'll try to be faster about my review next cycle – sorry for the delay.
Attachment #8794073 - Flags: review?(michael.l.comella) → feedback+
Attachment #8727203 - Attachment is obsolete: true
By the way, baishya, thanks for the patch and welcome to Bugzilla! :)
Hey, is this bug still being worked on :) ?
Baishya, are you still working on this? You were making good progress, think we can close it out?
Flags: needinfo?(baishya.anuraag1920)
I was looking into this issue. I think Baishya has taken care of points 1 and 2 of comment 32. For the last one , in GeckoClickHelper, should I use getStringHelper() from UITestContext instead of StringHelper.get() ? 
Can someone please help me get started with my first issue ?
Flags: needinfo?(michael.l.comella)
Redirecting to ahunt – I'm no longer working on Fennec.
Flags: needinfo?(michael.l.comella) → needinfo?(ahunt)
(In reply to tarun14110 from comment #36)
> I was looking into this issue. I think Baishya has taken care of points 1
> and 2 of comment 32. For the last one , in GeckoClickHelper, should I use
> getStringHelper() from UITestContext instead of StringHelper.get() ? 
> Can someone please help me get started with my first issue ?

Looking at the existing patch is a good start, however I think the codebase has changed since when the patch was posted, so I'd recommend starting from scratch and applying the same changes.

And yes, it looks like you can get StringHelper directly from the existing UITestContext in GeckoClickHelper.
Flags: needinfo?(ahunt)
Attached patch 1158979_friedger.patch (obsolete) — Splinter Review
Created a new patch with the updated code base
Flags: needinfo?(ahunt)
Attachment #8862816 - Flags: review?(ahunt)
Is this issue still open ?Can I work on it?
Flags: needinfo?(michael.l.comella)
Redirecting to Nevin as I'm no longer working on Fennec.
Assignee: baishya.anuraag1920 → nobody
Mentor: liuche, michael.l.comella → cnevinchen
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(cnevinchen)
Flags: needinfo?(andrzej)
Hi Suneet.
Sorry for the late reply. Feel free to take it :)
Flags: needinfo?(cnevinchen) → needinfo?(suneetbond91)
:nechen ! plz guide me further and lets connect on IRC.
Flags: needinfo?(suneetbond91)
I'm a student at Seneca college learning open source, and I was hoping to work on this bug for my course.  If no one else is currently working on it, I'd like to give it a try.
Hey if this still available, I would like to give it a try. Thanks
Nevin, please advise the current state of this bug.
Flags: needinfo?(cnevinchen)
For those who are interested, please feel free to take it. You can reach me via :nechen on IRC or nechen@mozilla.com.
Please set needinfo? flag (with my email or IRC name) so I can get notified when you post a comment. Thank you!
Flags: needinfo?(suneetbond91)
Flags: needinfo?(fatehsandhu)
Flags: needinfo?(cnevinchen)
Flags: needinfo?(avedis777)
I want to take this issue, guide me!
Flags: needinfo?(suneetbond91)
Flags: needinfo?(cnevinchen)
Hi Suneet
Please let me know if you have problem with Mozreview :)
Flags: needinfo?(cnevinchen) → needinfo?(suneetbond91)
Hey Nevin, if this bug is still up for grabs, I'll take it. I have worked on some other bugs in the non-android mozilla-central builds, just not android, but I do have an android build as well.
Flags: needinfo?(nechen)
Please feel free to take it:)
Flags: needinfo?(nechen)
Awesome, thanks Nevin.
Hey Nevin, sorry about the long wait, I had some issues getting mach bootstrap to work and update the android sdk. I have a couple questions about testing in android if you don't mind answering. I have made changes and need to test them but I am not sure how it needs to be done. From what I can tell in the robocop.ini file, testActivityStreamPocketReferrer.java is a robocop test which from reading about robocop is done through './mach robocop <file name>'. 

Then there is BaseRobocopTest.java, GeckoClickHelper.java, and the StringHelper.java file itself. Is there a certain way I should test these other files, like mochitest? Just based on reading BaseRobocopTest.java and the test return type in that file, it looks like it is a mochitest. Thanks for the help.
Flags: needinfo?(cnevinchen)
Uses of StringHelper.get are replaced with the StringHelper instance in BaseRobocopTest.java and UITestContext.
Comment on attachment 8980947 [details] [diff] [review]
Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance

After posting the last comment about the tests, I saw that the BaseRobocopTest is the starting point for the tests so my last comment probably doesn't make any sense. I did test testActivityStreamPocketReferrer.java file however. I wanted to get some feedback here and see if this is about how StringHelper should be used.
Flags: needinfo?(cnevinchen)
Attachment #8980947 - Flags: feedback?(cnevinchen)
Comment on attachment 8980947 [details] [diff] [review]
Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance

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

Thanks for the patch. Please help fix and request a review.

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/GeckoClickHelper.java
@@ +16,4 @@
>      private static Solo sSolo;
>      private static Activity sActivity;
>      private static Driver sDriver;
> +    private static StringHelper mStringHelper;

I think it should be prefixed with 's'
Attachment #8980947 - Flags: feedback?(cnevinchen) → feedback+
Assignee: nobody → reeisesean
Uses of StringHelper.get are replaced with the StringHelper instance in BaseRobocopTest.java and UITestContext.
Attachment #8981228 - Flags: review?(cnevinchen)
Attachment #8980947 - Attachment is obsolete: true
Comment on attachment 8862816 [details] [diff] [review]
1158979_friedger.patch

># HG changeset patch
># User friedger <friedger@gmail.com>
># Date 1493380994 -7200
>#      Fri Apr 28 14:03:14 2017 +0200
># Node ID 6f698c745cb638815d6d9219b70efa29fec11d28
># Parent  d19b88fdf81d7dff1ac2ffcec4084eee7d666d11
>Bug 1158979 - Remove StringHelper.get and replace the StringHelper assignment with StringHelper.initialize
>
>diff --git a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java
>--- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java
>+++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/BaseRobocopTest.java
>@@ -24,7 +24,6 @@ import org.mozilla.gecko.FennecMochitest
> import org.mozilla.gecko.FennecNativeActions;
> import org.mozilla.gecko.FennecNativeDriver;
> import org.mozilla.gecko.FennecTalosAssert;
>-import org.mozilla.gecko.GeckoAppShell;
> import org.mozilla.gecko.GeckoProfile;
> import org.mozilla.gecko.updater.UpdateServiceHelper;
> 
>@@ -174,8 +173,7 @@ public abstract class BaseRobocopTest ex
>         // Set up Robotium.solo and Driver objects
>         Activity tempActivity = getActivity();
> 
>-        StringHelper.initialize(tempActivity.getResources());
>-        mStringHelper = StringHelper.get();
>+        mStringHelper = StringHelper.initialize(tempActivity.getResources());
> 
>         mSolo = new Solo(getInstrumentation(), tempActivity);
>         mDriver = new FennecNativeDriver(tempActivity, mSolo, mRootPath);
>diff --git a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/StringHelper.java b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/StringHelper.java
>--- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/StringHelper.java
>+++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/StringHelper.java
>@@ -423,17 +423,11 @@ public class StringHelper {
>         DONT_RESTORE_QUIT = res.getString(R.string.pref_restore_quit);
>     }
> 
>-    public static void initialize(Resources res) {
>+    public static StringHelper initialize(Resources res) {
>         if (instance != null) {
>             throw new IllegalStateException(StringHelper.class.getSimpleName() + " already Initialized");
>         }
>         instance = new StringHelper(res);
>-    }
>-
>-    public static StringHelper get() {
>-        if (instance == null) {
>-            throw new IllegalStateException(StringHelper.class.getSimpleName() + " instance is not yet initialized. Use StringHelper.initialize(Resources) first.");
>-        }
>         return instance;
>     }
> 
>diff --git a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/GeckoClickHelper.java b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/GeckoClickHelper.java
>--- a/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/GeckoClickHelper.java
>+++ b/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/helpers/GeckoClickHelper.java
>@@ -16,11 +16,13 @@ public class GeckoClickHelper {
>     private static Solo sSolo;
>     private static Activity sActivity;
>     private static Driver sDriver;
>+    private static StringHelper sStringHelper;
> 
>     protected static void init(final UITestContext context) {
>         sSolo = context.getSolo();
>         sActivity = context.getActivity();
>         sDriver = context.getDriver();
>+        sStringHelper = context.getStringHelper();
>     }
> 
>     private GeckoClickHelper() { /* To disallow instantiation. */ }
>@@ -35,7 +37,7 @@ public class GeckoClickHelper {
>         openLinkContextMenu();
> 
>         // Click on "Open Link in New Tab"
>-        sSolo.clickOnText(StringHelper.get().CONTEXT_MENU_ITEMS_IN_NORMAL_TAB[0]);
>+        sSolo.clickOnText(sStringHelper.CONTEXT_MENU_ITEMS_IN_NORMAL_TAB[0]);
>     }
> 
>     /**
>@@ -48,7 +50,7 @@ public class GeckoClickHelper {
>         openLinkContextMenu();
> 
>         // Click on "Open Link in New Private Tab"
>-        sSolo.clickOnText(StringHelper.get().CONTEXT_MENU_ITEMS_IN_NORMAL_TAB[1]);
>+        sSolo.clickOnText(sStringHelper.CONTEXT_MENU_ITEMS_IN_NORMAL_TAB[1]);
>     }
> 
>     private static void openLinkContextMenu() {
Attachment #8862816 - Attachment is obsolete: true
Attachment #8862816 - Flags: review?(andrzej)
Attachment #8794073 - Attachment is obsolete: true
The patch looks look, could you please submit a patch using MozReview so I can run try build on it?
Thank you!
Flags: needinfo?(reeisesean)
Flags: needinfo?(reeisesean)
Status: NEW → ASSIGNED
(In reply to Nevin Chen [:nechen] from comment #59)
> The patch looks look, could you please submit a patch using MozReview so I
> can run try build on it?
> Thank you!

Done! Hopefully I did this correctly, it was the first time I pushed to MozReview.
Comment on attachment 8981228 [details] [diff] [review]
Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance

will use the one via mozreview
Attachment #8981228 - Attachment is obsolete: true
Attachment #8981228 - Flags: review?(cnevinchen)
Comment on attachment 8983660 [details]
Bug 1158979 - Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance.

Thank you!
Attachment #8983660 - Flags: review?(cnevinchen) → review+
(if you haven't done so) Please request for Level 1 access so you can start a try build by yourself.
https://www.mozilla.org/en-US/about/governance/policies/commit/
Feel free to ni me if you need someone to vouch for you.
Thank you!
Flags: needinfo?(reeisesean)
This still needs to be approved for landing on mozzreview in order to be pushed by the sherrifs on duty. Thanks
Comment on attachment 8983660 [details]
Bug 1158979 - Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance.

https://reviewboard.mozilla.org/r/249494/#review256100
(In reply to Nevin Chen [:nechen] from comment #64)
> (if you haven't done so) Please request for Level 1 access so you can start
> a try build by yourself.
> https://www.mozilla.org/en-US/about/governance/policies/commit/
> Feel free to ni me if you need someone to vouch for you.
> Thank you!

I have not yet, that would be awesome. I will work on putting in a request. 

And it looks like you already changed the r? to r+ as well in mozreview. Thanks for the help Nevin.
Flags: needinfo?(reeisesean)
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/51086f4622b3
Removed StringHelper.get and changed StringHelper.initialize to return the StringHelper instance. r=nechen
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/51086f4622b3
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.