Use resource prefix for library resources #796

WillieCubed · 2017-07-10T20:39:17Z

This fixes #794.

…nflation

samtstern · 2017-07-10T20:42:49Z

auth/src/test/java/com/firebase/ui/auth/ui/phone/SpacedEditTextTest.java

@@ -52,9 +52,9 @@ public void setUp() throws Exception {
        context = mock(Context.class);
        array = mock(TypedArray.class);

-        when(array.getFloat(R.styleable.SpacedEditText_spacingProportion, 1)).thenReturn
+        when(array.getFloat(R.styleable.fui_SpacedEditText_fui_spacingProportion, 1)).thenReturn


Any idea why this one go a double fui?

I think that's an effect of the build tools appending the two resource names together. I'll make sure it's reduced to SpacedEditText_fui_spacingProportion, if that's okay.

SUPERCILEX · 2017-07-10T21:06:41Z

database/src/main/java/com/firebase/ui/database/FirebaseListAdapter.java

@@ -27,21 +29,21 @@
 public abstract class FirebaseListAdapter<T> extends BaseAdapter implements FirebaseAdapter<T> {
    private static final String TAG = "FirebaseListAdapter";

-    protected final Activity mActivity;
+    protected final Context mContext;


Can we remove the DB changes since they aren't related to this PR? Otherwise LGTM! 😀

I think those will be fixed after the merge conflicts are fixed, those changes have already been accepted in another PR.

@samtstern yeah, once I saw the conflicts I realized this PR was built using personal commits instead of dev. Hopefully it won't be too tough to resolve them! 😀

Yeah, I'm still learning how to use git. Will do.

SUPERCILEX · 2017-07-10T21:08:29Z

auth/src/main/java/com/firebase/ui/auth/ui/phone/CountryListAdapter.java

@@ -38,7 +38,7 @@
    private String[] sections;

    public CountryListAdapter(Context context) {
-        super(context, R.layout.dgts__country_row, android.R.id.text1);
+        super(context, R.layout.fui_dgts__country_row, android.R.id.text1);


While we're renaming, might as well remove that extra _.

SUPERCILEX · 2017-07-10T21:16:03Z

@TheCraftKid might I also suggest you try branching? 😂 Nah, I'm just kidding with you but you should always try starting a PR by checking out a fresh version of upstream/dev, or in this case the 2.1 dev branch. Otherwise you end up with a whole bunch of conflicts and unrelated change like that DB stuff. Best of luck resolving those conflicts! 😀

samtstern · 2017-07-13T21:29:56Z

@TheCraftKid we're getting close to 2.1.0 and I am happy to include this in the release if you have time to finish it in the next day or two. What do you think? If you have any questions or need some help just let me know.

WillieCubed · 2017-07-14T21:50:39Z

Yep, sorry about the delay; a Windows update messed up my desktop environment.

On a side note, I learned how to use branches and remove the irrelevant code to this PR. Is there a way for me to add my commits to this PR, or will I have to make a new one?

samtstern · 2017-07-14T22:04:31Z

@TheCraftKid just commit again and push to the same branch, the PR will update like magic.

…2.1.0-dev

WillieCubed · 2017-07-15T01:05:09Z

Wow, that took way too long. That updated like magic, all right. I'm really sorry if there are some screwy commits or anything else in this PR.

Final note: just to get the linter to stop complaining, I added tools:ignore="ResourceName" to the root of the strings.xml file. Just making sure this is okay.

SUPERCILEX · 2017-07-15T02:42:07Z

@TheCraftKid glad you pulled through!!! 😀

I hate to be the bearer of bad news again, but the issue you're trying to resolve—#794—is about the string res names, not the layouts. We definitely should keep the layout name changes (thanks!), but the styles.xml file should have a root res name prefix ignore tag instead of the string res which should each have the prefix. More work for you, sorry! 😊

SUPERCILEX · 2017-07-15T02:42:58Z

auth/src/main/res/values/config.xml

@@ -1,4 +1,4 @@
-<resources>
+<resources xmlns:tools="http://schemas.android.com/tools">


We should also ignore at root here.

WillieCubed · 2017-07-17T15:01:46Z

Okay, that should solve the issues @SUPERCILEX brought up.

samtstern · 2017-07-17T16:00:32Z

@TheCraftKid we still have a ton of merge issues though, you need to merge the firebase version-2.1.0-dev branch into your branch and then resolve all conflicts.

samtstern · 2017-07-17T16:17:29Z

@TheCraftKid most of these conflicts are my fault I will send a PR to your branch that will fix it.

Change-Id: Ib9f64e94983cb39e70a03b526be9bb25ead888d1

samtstern · 2017-07-17T16:32:39Z

Opened WillieCubed#1 which will fix it.

Here's an issue though: because this adds the fui_ prefix to all of the strings it messes up all of the translations. Obviously I could just add the fui_ prefix to all of our translated strings as well but the problem is in the translation portal we use the string name is used as a unique ID so I'm actually not sure how it would react to this refactoring.

Since the major goal of 2.1.0 was to get translations out I am concerned about how this PR affects it. I'd like to punt this to 2.1.1 so I can think about the implications of adding this prefix.

@TheCraftKid
@SUPERCILEX

what do you think?

WillieCubed · 2017-07-17T20:20:56Z

I'd be willing to keep v2.1.0 translation-based and move the resource name changes to v2.1.1 if that means we can push the essential v2.1.0 changes.

samtstern · 2017-07-17T20:33:48Z

@TheCraftKid thanks for your flexibility, I think that's the safest move. I definitely want to get your resource prefix change in ASAP after 2.1.0 though, it's the right thing to do for developers.

Fix merges for PR firebase#796

samtstern · 2017-07-17T21:59:40Z

Woot no more conflicts! The build is failing now but that's working as intended since we have not answered the translation question yet.

Change-Id: I7f7f848941c6482bf444dd743a05acc78aa0455c

Change-Id: I5dadda6cbe361ef756c4d3ba3525ca3eb76925d0

Change-Id: Iac2405e6a11ee35fb8d0c05afce35d8fa4315321

Change-Id: Ide75dce369798fd0b14ac58928123bba12bf762e

samtstern · 2017-07-19T17:03:40Z

@TheCraftKid I opened WillieCubed#2 to fix the final issues.

Finish PR firebase#796

samtstern · 2017-07-20T20:29:36Z

@TheCraftKid nice! This one is all set to merge, thanks for the hard work.

WillieCubed added 2 commits June 22, 2017 14:06

Update FirebaseListAdapter to use Context in constructor and layout i…

b767c8a

…nflation

Update resources to use library prefix "fui_"

ebd0213

samtstern reviewed Jul 10, 2017

View reviewed changes

SUPERCILEX reviewed Jul 10, 2017

View reviewed changes

WillieCubed added 3 commits July 14, 2017 18:15

Fix updating resources to use library prefix "fui_"

e792258

Merge remote-tracking branch 'origin/version-2.1.0-dev' into version-…

da352f4

…2.1.0-dev

Fix updating resource prefixes

58ac141

SUPERCILEX reviewed Jul 15, 2017

View reviewed changes

WillieCubed added 3 commits July 17, 2017 09:28

Simplify lint ignoring style resource names

1018d63

Simplify lint ignoring config.xml values

5766631

Update strings.xml to use "fui_" resource prefix

251f191

Merge branch 'version-2.1.0-dev' into pr-796

efca985

Change-Id: Ib9f64e94983cb39e70a03b526be9bb25ead888d1

Merge pull request #1 from samtstern/pr-796

7019195

Fix merges for PR firebase#796

samtstern mentioned this pull request Jul 18, 2017

Translations prefix scripts #807

Closed

samtstern added 3 commits July 18, 2017 12:56

Add new fui scripts

eeecac6

Change-Id: I7f7f848941c6482bf444dd743a05acc78aa0455c

Refactor string scripts

4aa3786

Change-Id: I5dadda6cbe361ef756c4d3ba3525ca3eb76925d0

Fix scripts, finish fui_ prefixing

1d13f72

Change-Id: Iac2405e6a11ee35fb8d0c05afce35d8fa4315321

samtstern added 2 commits July 19, 2017 09:58

Run clean up script, makes lint pass

1e4dbfd

Change-Id: Ide75dce369798fd0b14ac58928123bba12bf762e

Merge remote-tracking branch 'thecraftkid/version-2.1.0-dev' into pr-796

22b17e8

Merge pull request #2 from samtstern/pr-796

95dfb3c

Finish PR firebase#796

samtstern changed the base branch from version-2.1.0-dev to version-2.1.1-dev July 20, 2017 20:29

samtstern merged commit 0f55407 into firebase:version-2.1.1-dev Jul 20, 2017

SUPERCILEX mentioned this pull request Jul 22, 2017

Fix broken auto styling #815

Merged

		@@ -1,4 +1,4 @@
		<resources>
		<resources xmlns:tools="http://schemas.android.com/tools">

Use resource prefix for library resources #796

Use resource prefix for library resources #796

Uh oh!

Conversation

WillieCubed commented Jul 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WillieCubed Jul 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SUPERCILEX commented Jul 10, 2017

Uh oh!

samtstern commented Jul 13, 2017

Uh oh!

WillieCubed commented Jul 14, 2017

Uh oh!

samtstern commented Jul 14, 2017

Uh oh!

WillieCubed commented Jul 15, 2017

Uh oh!

SUPERCILEX commented Jul 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WillieCubed commented Jul 17, 2017

Uh oh!

samtstern commented Jul 17, 2017

Uh oh!

samtstern commented Jul 17, 2017

Uh oh!

samtstern commented Jul 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WillieCubed commented Jul 17, 2017

Uh oh!

samtstern commented Jul 17, 2017

Uh oh!

samtstern commented Jul 17, 2017

Uh oh!

samtstern commented Jul 19, 2017

Uh oh!

samtstern commented Jul 20, 2017

Uh oh!

Uh oh!

WillieCubed Jul 11, 2017 •

edited

Loading

SUPERCILEX commented Jul 15, 2017 •

edited

Loading

samtstern commented Jul 17, 2017 •

edited

Loading