-
Notifications
You must be signed in to change notification settings - Fork 401
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
Refactor groovy gradle files to kotlin dsl #2022
Refactor groovy gradle files to kotlin dsl #2022
Conversation
if len(tokens) == 2 and tokens[0] == 'minSdkVersion': | ||
return int(tokens[1]) | ||
if len(tokens) == 3 and tokens[0] == 'minSdk': | ||
return int(tokens[2]) |
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.
The line that contains the minSdk
has now three tokens.
Before:
minSdkVersion 21
Now:
minSdk = 21
@@ -3,7 +3,7 @@ | |||
set -e | |||
|
|||
NEW_VERSION_NAME=$1 |
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.
awk now needs to print the 3rd element as there is now an equals sign in the middle for versionName
and versionCode
, like:
versionCode = 4372
versionName = "1.27.0-rc.2"
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.
Bumped to version 1.27.2-rc.2
now
NEW_VERSION_CODE=$(($OLD_VERSION_CODE + 1)) | ||
sed -i -e "s/versionCode $OLD_VERSION_CODE/versionCode $NEW_VERSION_CODE/" "app/build.gradle" | ||
sed -i -e "s/versionCode = $OLD_VERSION_CODE/versionCode = $NEW_VERSION_CODE/" "app/build.gradle.kts" |
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.
When replacing the old version code with the new one we now need to add the =
sign in the middle otherwise sed won't find the line we're trying to update.
app/build.gradle.kts
Outdated
|
||
signingConfigs { | ||
create("release") { | ||
storeFile = file(System.getenv("SYNCTHING_RELEASE_STORE_FILE") ?: "release.keystore") |
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.
That was null
before instead of "release.keystore" - how is that equivalent?
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.
you're correct, I referenced my own config while writing this 😓
I updated the PR to set storeFile
to null
instead if the SYNCTHING_RELEASE_STORE_FILE
var is not set
app/build.gradle.kts
Outdated
isMinifyEnabled = false | ||
} | ||
getByName("release") { | ||
signingConfig = signingConfigs.getByName("release") |
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.
Previously this conditionally pointed to either the store-file within the release config, or the release config itself. Previous state is not at all self-explanatory, so if you know/think the new way is right I am willing to just take that at face value and try.
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.
I made the wrong assumption that the signingConfig
should never be null for the release
build type.
I updated the logic to result in a null signingConfig
in case there is no storeFile
available to be the same as in previous config.
That was quite annoying to review with all the lines "changed", thus no usable diff, and needed to be checked that they actually didn't change semantically xD One question about the signing store-file, otherwise lgtm. |
aaace13
to
a75496d
Compare
Sorry for any confusion and thanks for the in-depth review 😉 |
Oh nothing to be sorry for, I didn't mean to complain about anything you have done. It was simply the very nature of the change that made it hard to review. |
## Changes * get rid of the JCenter repository * centralize repository declaration for all projects in project settings * refactor `build.gradle` and `settings.gradle` from Groovy syntax to Kotlin DSL (thus replacing those files with `build.gradle.kts` and `settings.gradle.kts` respectively) The remaining `build.gradle` files are refactored in: #2022
Description:
Refactor gradle build files to use Kotlin DSL instead of Groovy.
There were also a bash script and a python script that needed to be updated because they relied on parsing the
build.gradle
files written in Groovy.This PR completes the work started in: #2000