1405124 - Version GeckoView AAR correctly

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

We hard code versionCode to 1, and versionName to "0.0.1".

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1405124 - Set appropriate version code and string for GeckoView AAR

Review commit: https://reviewboard.mozilla.org/r/185830/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185830/

Comment 2

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8914503 [details]
Bug 1405124 - Set appropriate version code and string for GeckoView AAR

https://reviewboard.mozilla.org/r/185830/#review194224

General idea looks good; r- just to make sure I look more closely later.

::: mobile/android/geckoview/build.gradle:5
(Diff revision 1)
>  buildDir "${topobjdir}/gradle/build/mobile/android/geckoview"
>  
>  apply plugin: 'com.android.library'
>  
> -def VERSION_NAME = '0.0.1'
> +def computeVersionCode() {

Include a comment explaining your scheme.  You're only giving yourself 100 versions, and we're at 57 -- that seems unwise.  Make it 1000.

Also, include an example `MOZ_APP_VERSION` or two, lik: // Split "57.0.1a2" into [57, 0, 1].  Can we make the "a1" thing more robust?  Is it always there and always length 2?

::: mobile/android/geckoview/build.gradle:163
(Diff revision 1)
>          description = "Generate Javadoc for build variant $name"
>          failOnError = false
>          destinationDir = new File(destinationDir, variant.baseName)
>          source = files(variant.javaCompile.source)
>          classpath = files(variant.javaCompile.classpath.files) + files(android.bootClasspath)
> -        options.windowTitle("Mozilla GeckoView Android API $VERSION_NAME Reference")
> +        options.windowTitle("Mozilla GeckoView Android API $mozconfig.substs.MOZ_APP_VERSION Reference")

I think this may not have been correct before -- don't we need `${...}`?  (Here and below.)

::: mobile/android/geckoview/build.gradle:192
(Diff revision 1)
>  apply plugin: 'maven'
>   
>  uploadArchives {
>      repositories.mavenDeployer {
>          pom.groupId = 'org.mozilla'
> -        pom.artifactId = 'geckoview'
> +        pom.artifactId = "geckoview-$mozconfig.substs.ANDROID_CPU_ARCH"

Hmm.  OK.

::: mobile/android/geckoview/build.gradle:192
(Diff revision 1)
>  apply plugin: 'maven'
>   
>  uploadArchives {
>      repositories.mavenDeployer {
>          pom.groupId = 'org.mozilla'
> -        pom.artifactId = 'geckoview'
> +        pom.artifactId = "geckoview-$mozconfig.substs.ANDROID_CPU_ARCH"

Can we bake the branch in here somehow too?  So you have `geckoview-{release,beta,nightly,snapshot}` in some way?

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8914503 [details]
Bug 1405124 - Set appropriate version code and string for GeckoView AAR

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/185830/diff/1-2/

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8914503 [details]
Bug 1405124 - Set appropriate version code and string for GeckoView AAR

https://reviewboard.mozilla.org/r/185830/#review195044

lgtm.  nits only.

::: mobile/android/geckoview/build.gradle:8
(Diff revisions 1 - 2)
>  apply plugin: 'com.android.library'
>  
> +// This converts MOZ_APP_VERSION into an integer
> +// version code.
> +//
> +// We take something like 58.1.2a1 and come out with 5800102

mild preference for the same base (1000) for each multiplication.

::: mobile/android/geckoview/build.gradle:10
(Diff revisions 1 - 2)
> +// This converts MOZ_APP_VERSION into an integer
> +// version code.
> +//
> +// We take something like 58.1.2a1 and come out with 5800102
> +// This gives us 3 digits for the major number, and 2 digits
> +// each for the minor and build number. Beta and Release

nit: full sentences.  Why the trailing "Beta and Release"?

::: mobile/android/geckoview/build.gradle:71
(Diff revisions 1 - 2)
>          buildConfigField 'String', "MOZ_APP_NAME", "\"${mozconfig.substs.MOZ_APP_NAME}\"";
>          buildConfigField 'String', "MOZ_APP_VENDOR", "\"${mozconfig.substs.MOZ_APP_VENDOR}\"";
>          buildConfigField 'String', "MOZ_APP_VERSION", "\"${mozconfig.substs.MOZ_APP_VERSION}\"";
>          buildConfigField 'String', "MOZ_APP_DISPLAYNAME", "\"${mozconfig.substs.MOZ_APP_DISPLAYNAME}\"";
>          buildConfigField 'String', "MOZ_APP_UA_NAME", "\"${mozconfig.substs.MOZ_APP_UA_NAME}\"";
> +        buildConfigField 'String', "MOZ_UPDATE_CHANNEL", "\"${mozconfig.substs.MOZ_UPDATE_CHANNEL}\"";

This isn't required, although you can add it to GeckoView if you want.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d985d6be2d1b
Set appropriate version code and string for GeckoView AAR r=nalexander

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d985d6be2d1b