-
Notifications
You must be signed in to change notification settings - Fork 119
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
VPN-6290 - Stop using the glean sdk generator script #9326
Conversation
52fbc57
to
9646c12
Compare
900ce15
to
15f3abb
Compare
@strseb I am assuming now we don't need to run gradle twice in cmake.sh? From local testing it seems to work -- the apk was created fine with just |
@@ -52,6 +41,8 @@ android { | |||
minSdkVersion Config.minSdkVersion | |||
targetSdkVersion Config.targetSdkVersion | |||
|
|||
buildConfigField "int", "VERSION_CODE", System.getenv("VERSIONCODE") ?: "99999" | |||
buildConfigField "String", "VERSION_NAME", '"' + (System.getenv("SHORTVERSION") ?: "") + '"' | |||
buildConfigField "String", "VERSIONCODE" , '"' +System.getenv("VERSIONCODE") + '"' |
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.
Why twice?
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.
Oh, one is "VERSIONCODE" and the other is "VERSION_CODE" 👀
It's good you commented here. It reminds me I had a little issue with these variables.
You and the internet had told me I should use versionName
and versionCode
here and they would set these variables. That didn't work though, that is why I used buildConfigField
here instead.
Also, I set these in a bunch of places, but I could not figure out how to just set it once. I don't like the way it is right now :(
aae3f96
to
fed0f15
Compare
dependencyResolutionManagement { | ||
versionCatalogs { | ||
create("gleanLibs") { | ||
from(files("$gleanVendoredPath/gradle/libs.versions.toml")) |
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.
Neeeat
591a944
to
1823996
Compare
1823996
to
05d0dad
Compare
We should do the same for Android. Let's do it in a separate PR because this is currently breaking CI IIUC.Doing Android here since fixing things for iOS broke them for Android.