-
Notifications
You must be signed in to change notification settings - Fork 29
feat: support flavors for JSBundle #163
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
Conversation
...ugins/react/brownfield/src/main/kotlin/com/callstack/react/brownfield/plugin/RNSourceSets.kt
Outdated
Show resolved
Hide resolved
hurali97
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR 🚀
...ugins/react/brownfield/src/main/kotlin/com/callstack/react/brownfield/plugin/RNSourceSets.kt
Outdated
Show resolved
Hide resolved
artus9033
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! What remains is just the uppercase var name we need to sort out, otherwise LGTM!
hurali97
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for the PR 🚀
|
Based on our private conversation with @hurali97, a decision was made to generate and include the JS bundle for both Debug and Release variants. Hence, I have reverted my commits |
60a2b89 to
c7d05c5
Compare
hurali97
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Currently,
brownfield-gradle-plugindoesn't support flavors. Only the default variants:DebugandReleaseare supported.If a different flavor is added, like:
FlavorOne, thecreateBundleReleaseJsAndAssetstask won't be executed due to it having a different name now:createBundleFlavorOneReleaseJsAndAssetsthus not generating theindex.android.bundle.Moreover, the assets source location for
index.android.bundleis also not correctly specified. It should reflect the new flavor:app/build/generated/assets/createBundleFlavorOneReleaseJsAndAssets/instead of the default variant:app/build/generated/assets/createBundleReleaseJsAndAssets/.Summary
This PR makes the
createBundle...JsAndAssetstask and assets source location dynamic, reflecting the actual flavor + build type.Test plan
These changes has been tested locally using maven local in a React Native brownfield project.