Skip to content
This repository was archived by the owner on May 20, 2025. It is now read-only.

Correctly outputs CodePushHash.json to custom folder #712

Merged
merged 1 commit into from
Feb 25, 2017

Conversation

comigor
Copy link
Contributor

@comigor comigor commented Feb 21, 2017

Remove assetsDir variable, as generateBundledResourcesHash.js should output CodePushHash.json to jsBundleDir${targetName} folder, and not the hardcoded $buildDir/intermediates/assets/${targetPath} one.

Remove `assetsDir` variable, as `generateBundledResourcesHash.js` should output `CodePushHash.json` to `jsBundleDir${targetName}` folder, and not the hardcoded `$buildDir/intermediates/assets/${targetPath}` one.
@msftclas
Copy link

Hi @Igor1201, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@comigor comigor changed the title remove assetsDir variable Correctly outputs CodePushHash.json to custom folder Feb 21, 2017
@richardhuaaa
Copy link
Contributor

@Igor1201: Apologies if my understanding is weak here - we can override where it is written to, but won't it still be read from the same location?

@comigor
Copy link
Contributor Author

comigor commented Feb 23, 2017

@Silhouettes yeah, once the app is built, it will be read from the same location (the assets folder inside the app), but my change was to set the folder in which the CodePushHash.json is generated before the app itself is built.

In my knowledge, we should fallback both paths to their default locations (build/intermediates/assets and build/intermediates/resources) but they should be overridable by the react config vars (resourcesDirDebug, resourcesDirRelease, jsBundleDirDebug and jsBundleDirRelease), exactly the way we're handling resourcesDir right now.

Copy link
Contributor

@richardhuaaa richardhuaaa left a comment

Choose a reason for hiding this comment

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

Okay - looks good, and thanks for the contribution!

@richardhuaaa richardhuaaa merged commit c794c36 into microsoft:master Feb 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants