-
Notifications
You must be signed in to change notification settings - Fork 354
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 billing and infrastructure flavor dimensions #4684
Add billing and infrastructure flavor dimensions #4684
Conversation
DROID-18 Improve gradle variant configuration
The current gradle configuration is not suitable for a multi-project setup and also doesn't differentiate between Google Play and GitHub builds. We should use: Build types: Flavors: |
c0d7bce
to
006bb93
Compare
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.
Reviewed 33 of 33 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @albin-mullvad)
android/lib/build.gradle.kts
line 1 at r1 (raw file):
I guess this was part of the formatting, but this looks a bit odd?
android/test/build.gradle.kts
line 1 at r1 (raw file):
Same as above
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.
Reviewable status: complete! all files reviewed, all discussions resolved
android/lib/build.gradle.kts
line 1 at r1 (raw file):
Previously, Pururun (Jonatan Rhodin) wrote…
I guess this was part of the formatting, but this looks a bit odd?
Yes, part of the formatting, but I agree
android/test/build.gradle.kts
line 1 at r1 (raw file):
Previously, Pururun (Jonatan Rhodin) wrote…
Same as above
Yes, part of the formatting, but I agree
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/StringExtensions.kt
line 5 at r1 (raw file):
import net.mullvad.mullvadvpn.constant.IS_PLAY_BUILD fun String.appendHideNavOnReleaseBuild(): String =
I saw that this was just moved and that the name hasn't changed but maybe we should rename this to appendHideNavOnPlayBuild
or something along those lines?
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.
Reviewable status: 29 of 33 files reviewed, 1 unresolved discussion (waiting on @Pururun and @Rawa)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/util/StringExtensions.kt
line 5 at r1 (raw file):
Previously, Rawa (David Göransson) wrote…
I saw that this was just moved and that the name hasn't changed but maybe we should rename this to
appendHideNavOnPlayBuild
or something along those lines?
Good catch! Done 🙂
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.
Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)
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.
Reviewed 29 of 33 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion
dd2742c
to
cc4d7f3
Compare
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.
Reviewed 29 of 33 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)
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.
Dismissed @Rawa from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved
Flavor dimension usage: * `billing` - used to build separate artifacts where (1) different policies are followed and (2) different dependencies are used. * `infrastructure` - used to improve development and testing against non-production environments.
This fix makes sure that both `*.play.aab` and `*.aab` bundles are supported and get the correct names. Ideally the `sed` command which this commit touches would be able to add the suffix using a small tweak to the pattern like this: `(MullvadVPN-.*-dev-.*)(\.apk|\.play\.aab|\.aab)` However, that pattern would result in the suffix being incorrectly placed due the greedy nature of `sed` like the "incorrect" one below: MullvadVPN-<version>.play+suffix.aab (incorrect) MullvadVPN-<version>+suffix.play.aab (correct)
cc4d7f3
to
62d94a2
Compare
This PR changes the build configuration from only relying on build types to also rely on two flavor dimensions.
Flavor dimensions:
billing
- used to build separate artifacts where (1) different policies are followed and (2) different dependencies are used. This dimension will initially have the following flavors:oss
,play
.infrastructure
- used to enable development and testing against non-production environments. This dimension will initially have the following flavors:prod
,devmole
.This is the full set of enabled variants:
TODO:
This change is