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

[Android][Improvement] Improve support for non debug/release variants #34678

Closed
wants to merge 1 commit into from

Conversation

sidferreira
Copy link

The current change allows projects to use different variants, staging in our case, properly.

Summary

In one of our projects, we have a third variant which is Staging. Without this change, our staging builds will crash with #33120

Changelog

[Android] [Added] - Add conditionals for releases that are not named debug/release

Test Plan

the current change allows projects to use different variants, staging in our case, properly.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Sep 13, 2022
@sidferreira sidferreira changed the title Improve support for non debug/release variants [Android][Improvement] Improve support for non debug/release variants Sep 13, 2022
@react-native-bot react-native-bot added Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Sep 13, 2022
@danilobuerger
Copy link
Contributor

Does this also fix #34686?

@sidferreira
Copy link
Author

@danilobuerger it looks like, but I wasn't familiar with that issue (I basically fixed it locally and suggested a PR)

Copy link
Contributor

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Thanks for your PR @sidferreira

I don't think we're going to merge this. The reason is that the deleteDebugFilesForVariant is already customizable. Users can specify a config.deleteDebugFilesForVariant

Your change will unnecessarily extend the API surface. Moreover it relies on Groovy dynamic property resolution, which is well supported in Kotlin. If we were to include this, we would have to extend also the react-native-gradle-plugin. As mentioned, I don't think we're going to do so.

I'm closing this for the aformentioned reasons.
Happy to discuss further if you wish 👍

@cortinico cortinico closed this Sep 20, 2022
@cortinico
Copy link
Contributor

Does this also fix #34686?

@danilobuerger Nope it does not. The changes are related but there is no fix needed for #34686

@sidferreira
Copy link
Author

@cortinico make sense. The only reason I dis it is cause other similar functions have the same pattern, it sounded like the best approach.

I'm just not sure there's a guide on how to add extra variants support...

@cortinico
Copy link
Contributor

I'm just not sure there's a guide on how to add extra variants support...

Nope there is no guide sadly. We're moving to use the React Native Gradle Plugin which will have a better variant support instead of doing string matching. We'll provide docs for it once it lands 👍

@sidferreira sidferreira deleted the patch-2 branch September 20, 2022 15:18
@sidferreira
Copy link
Author

@cortinico thanks for your work and your answers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: Android Android applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants