-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Issue #60: Generate versionCode for releases dynamically. #62
Conversation
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.
LGTM overall. I think we should address the timezone and the architecture digit before landing. What do you think?
buildSrc/src/main/java/AppVersion.kt
Outdated
// | ||
// For September 10th, 2018, 5:04pm am this will generate the versionCode: 2531704 (0-253-1704). | ||
|
||
val today = Date() |
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.
How about using java.time.Instant.now()
? This package has been part of Java since Java 8. It gives the UTC time https://docs.oracle.com/javase/8/docs/api/java/time/Instant.html#now--
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.
Changed in favor of ZonedDateTime.now(ZoneOffset.UTC)
. java.time.Instant.now()
only returned nano/milli/seconds from Epoch. ZonedDateTime is more handy
buildSrc/src/main/java/AppVersion.kt
Outdated
val today = Date() | ||
|
||
// We use the current year (double digit) and subtract the base year (see above). This value | ||
// will start counting at 0 and increment by one every year. |
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.
👍
buildSrc/src/main/java/AppVersion.kt
Outdated
|
||
// We append the hour in day (24h) and minute in hour (7:26 pm -> 1926). We do not append | ||
// seconds. This assumes that we do not need to build multiple release(!) builds the same | ||
// minute. |
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 assumption seems fair to me, for now.
app/build.gradle
Outdated
// Otherwise this will create version code conflicts. | ||
|
||
if (variant.flavorName.contains("X86")) { | ||
versionCode = versionCode + 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.
Should we use a digit to highlight what architecture it is? I'm worried that 2531704
means nothing architecture-wise. This can be confusing on the Google Play dashboard. How about we have:
25317040
-> arm25317041
-> arm6425317042
-> x86
What do you think?
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 implemented the proposal.
|
||
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner" | ||
} | ||
|
||
buildTypes { | ||
release { | ||
minifyEnabled false | ||
shrinkResources false |
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.
Out of curiosity, what does this config do?
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.
It's purpose is to make the APK as small as possible e.g. by removing resources that are not being used:
https://developer.android.com/studio/build/shrink-code
This PR blocks bug 1491796. Could you tell me what you think of it @csadilek ? :) |
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.
🚢
|
||
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner" | ||
} | ||
|
||
buildTypes { | ||
release { | ||
minifyEnabled false | ||
shrinkResources false |
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.
It's purpose is to make the APK as small as possible e.g. by removing resources that are not being used:
https://developer.android.com/studio/build/shrink-code
@JohanLorenzo ok done :) |
No description provided.