-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add more config parameters for android_build #34
Add more config parameters for android_build #34
Conversation
Add more parameters to the android_build command. This allows us to be more explicit with our build variants and it terms will save minutes on circleCI by only specifying the build variantes that we really needs fixes react-native-community#33
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 sending this in. Apologies for the delay in reviewing it.
I'm wondering if this could be achieved without a breaking change. To do this, you could add a new flavour
parameter, which defaults to an empty string, and then using it in the command like this:
"cd <<parameters.project_path>> && chmod +x gradlew && ./gradlew --build-cache --max-workers 2 --continue assemble<<parameters.flavour>><<parameters.build_type>> assemble<<parameters.flavour>>AndroidTest -DtestBuildType=<<parameters.build_type>> --stacktrace"
You should then be able to achieve what you need and only build a single set of APKs, but would introducing a breaking change in the Orb. Would that work for you?
Hm, that is not a bad idea. It could work for my case. I was however thinking further on this and I don't really like my PR in the first place. After submitting it I started thinking that it would be much better if we had just
What we have right now. And then we could override it with whatever we want. I know this is again a breaking change, but it allows better flexibility with less params. What do you think? |
The commands in this Orb are meant to cover the basic use cases, with some customisation options so it's flexible enough to use with most apps. The idea isn't to be infinitly flexible. If a command doesn't offer the flexibility that someone needs, they can always just write a custom step and ignore the library one. |
@matt-oakes that's a fair point and I think that by changing the above it will work with "most apps" as you said. I know that I can copy it and modify it, but I still think that if we could provide something that works for "most apps" it will improve developer productivity. It's much better to configure a line, than to copy 50 just because one can't configure a line... I think that gradlew_params would be the most versatile approach and will cover most use cases. It's up to you. If you don't want this, I'll close the PR. |
@matt-oakes I just realized that this PR status is "change requested". So what do you want me to do? I personally am in favor of gradlew_params. But I feel that flavor is not making it better. Tomorrow someone will need yet another param and when do we say enough is enough? On the other hand gradlew_params let's users do their thing... |
I see what you mean about making it so people aren't required to break out from the command. Could we do both for the best flexibility? If you added in the You could then also include a Would that work or is that just complicating things even more? |
You mean like <<parameters.additional_gradlew_params>>:
or if gradlew_params set - ignore all other params and just use gradlew_params? |
I think additional makes more sense. If they want to override the whole thing then they may as well just not use the command and write their own. |
@matt-oakes just started implementing this and realized that it won't work the way you are suggesting.
just adding a flavor variable won't cut it. The command right now is:
if I add flavor=Develop then I would end with I'm giving up on this. I'll copy the whole android_build job in our project and just modify the build step there. |
@matt-oakes is it necessary to run Gradle task I would like to help and fix this issue as we use product flavors |
Summary
Add more parameters to the android_build command. This allows us to be more explicit with our build variants and it terms will save minutes on circleCI by only specifying the build variantes that we really needs
fixes #33
This is a breaking change!
We need to specify the following parameters
where previously we only had build_type.
If we don't specify any parameters then the config should have the old behavior.
I also updated the android_build job with the new syntax
Checklist
README.md
CHANGELOG.md
example/App.js
)