[TIMOB-25032] iOS: Resolve asset-catalog hashes back to real names, fail build when detecting duplicates #10239
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tests:
Generated by 🚫 dangerJS |
hansemannn
requested changes
Aug 8, 2018
iphone/Classes/FilesystemModule.m
Outdated
| RELEASE_TO_NIL(sha) | ||
| } | ||
| } | ||
| NSString *fileName = [imageArg lastPathComponent]; |
There was a problem hiding this comment.
I think you can reuse the existing imageArg property:
imageArg = [imageArg lastPathComponent];and align it to line 198 to be right after the other string manipulations. Same for other occurences of this code in the core.
iphone/cli/commands/_build.js
Outdated
| const checked = new Set(); | ||
| const duplicates = new Set(); | ||
|
|
||
| Object.keys(imageAssets).forEach(function (assetName) { |
There was a problem hiding this comment.
Maybe some ES6 here?
Object.keys(imageAssets).forEach(assetName => {
iphone/cli/commands/_build.js
Outdated
| }); | ||
|
|
||
| if (duplicates.size > 0) { | ||
| duplicates.forEach(function (asset) { |
iphone/cli/commands/_build.js
Outdated
|
|
||
| if (duplicates.size > 0) { | ||
| duplicates.forEach(function (asset) { | ||
| this.logger.error(__('Image asset name "%s" used multiple time, duplicate names are not permitted', asset)); |
There was a problem hiding this comment.
this.logger.error(__('Image asset name "%s" used multiple time, asset names should be unique in Titanium SDK 8.0.0+', asset));
hansemannn
approved these changes
Aug 8, 2018
…ail build when detecting duplicates
Rebased with master to resolve existing merge conflicts BREAKING CHANGE: Will throw an error on build if there are duplicated image names in the project
Changed the warning message from notifying of 8.0.0 to 9.0.0
|
cb1kenobi
reviewed
Jun 5, 2019
There was a problem hiding this comment.
I think this ticket is a bad idea and that we should NOT merge it. Please see my comment on the ticket: https://jira.appcelerator.org/browse/TIMOB-25032.
build
reviewed
Sep 9, 2019
| * Please see the LICENSE included with this distribution for details. | ||
| */ | ||
| /* eslint-env mocha */ | ||
| /* global Ti */ |
There was a problem hiding this comment.
- 🚫 tests/Resources/ti.ui.imageview.addontest.js line 8 – 'Ti' is already defined as a built-in global variable. (no-redeclare)
|
Closing in favor of PR #11437 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA: https://jira.appcelerator.org/browse/TIMOB-25032
Removed hashing from file names, first in the build step, so assets are stored under their real names, with checks in place to make sure duplicates are not used. And also in the module code to ensure de-hashing isn't attempted