Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-24834] Android: Merge value resources into single file #9162

Merged
merged 2 commits into from Jun 20, 2017

Conversation

janvennemann
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-24834

Optional Description:
Resource sets under the values folder can be split into different files. Those need to be merged back into a single values file so Android library resources and the app's resources can be properly merged together.

See: https://android.googlesource.com/platform/tools/base/+/android-7.1.1_r43/sdk-common/src/main/java/com/android/ide/common/res2/MergedResourceWriter.java

Resource sets under the values folder can be split into different files. Those need to be merged back into a single values file so Android library resources and the app's resources can be properly merged.

See: https://android.googlesource.com/platform/tools/base/+/android-7.1.1_r43/sdk-common/src/main/java/com/android/ide/common/res2/MergedResourceWriter.java
@janvennemann janvennemann changed the title [TIMOB-24834] Merge value resources into single file [TIMOB-24834] Android: Merge value resources into single file Jun 20, 2017
@@ -54,6 +54,16 @@ AndroidBaseBuilder.prototype.writeXmlFile = function writeXmlFile(srcOrDoc, dest
}
fs.writeFileSync(dest, '<?xml version="1.0" encoding="UTF-8"?>\n' + srcDoc.toString());
return;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I prefer to put the truthy block first. It makes the code easier to read.

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (did CR, not FR)

@hansemannn hansemannn added this to the 6.1.1 milestone Jun 20, 2017
@hansemannn
Copy link
Collaborator

@janvennemann Backport please

@mukherjee2 mukherjee2 self-requested a review June 20, 2017 17:08
Copy link
Contributor

@mukherjee2 mukherjee2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed FR!
Node Version: 6.10.3
NPM Version: 3.10.10
Mac OS: 10.12.4
Appc CLI: 6.2.2
Appc CLI NPM: 4.2.9
Titanium SDK version: from PR/9163
Appcelerator Studio, build: 4.9.0.201705302345
Xcode 8.3.2
Hyperloop 2.1.1
Android 7.1 Device

I built with SDK 6.1.0 and reproduced the original bug. Then I built with PR/9163 and found that the errors are no longer there, and the app build to device also worked as expected.

@mukherjee2 mukherjee2 merged commit 5a7b025 into tidev:master Jun 20, 2017
@hansemannn hansemannn modified the milestones: 6.2.0, 6.1.1 Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants