Skip to content
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

Use one version code for all apks #503

Merged
merged 4 commits into from
Aug 8, 2023
Merged

Use one version code for all apks #503

merged 4 commits into from
Aug 8, 2023

Conversation

linhpha
Copy link
Collaborator

@linhpha linhpha commented Aug 7, 2023

We can move away from having separate version code for each build type now that we're publishing app bundle

This change should also help resolve issues with AGP 8.1 upgrade

@github-actions
Copy link

github-actions bot commented Aug 7, 2023

  1. The code can be improved by removing the unnecessary use of variantOutput.abiString and VERSION_CODES map. Instead, a constant value can be used for the version code based on the ABI.
internal class ApkVersioningPlugin : Plugin<Project> {
    override fun apply(project: Project) {
        project.androidVariants.all { variant ->
            variant.outputs.all { output ->
                output.versionCode.setDisallowChanges(ApkVersioning.VERSION_CODE)
            }
        }
    }
}

private object ApkVersioning {
    const val VERSION_CODE: Int = 90000000 // Set the desired version code here
}
  1. If you still want to use the VERSION_CODES map, you can simplify it by using a when statement instead of a map.
private object ApkVersioning {
    const val VERSION_CODE: Int = 90000000 // Set the desired version code here

    fun getVersionCode(abiString: String?): Int {
        return when (abiString) {
            "arm64-v8a" -> VERSION_CODE + 3
            "armeabi-v7a" -> VERSION_CODE + 2
            "x86" -> VERSION_CODE + 8
            "x86_64" -> VERSION_CODE + 9
            else -> VERSION_CODE
        }
    }
}

internal class ApkVersioningPlugin : Plugin<Project> {
    override fun apply(project: Project) {
        project.androidVariants.all { variant ->
            variant.outputs.all { output ->
                output.versionCode.setDisallowChanges(ApkVersioning.getVersionCode(output.abiString))
            }
        }
    }
}
  1. If the VERSION_CODE is always the same, you can simplify the code further by directly setting the version code without using a separate object.
internal class ApkVersioningPlugin : Plugin<Project> {
    override fun apply(project: Project) {
        project.androidVariants.all { variant ->
            variant.outputs.all { output ->
                output.versionCode.setDisallowChanges(90000000) // Set the desired version code here
            }
        }
    }
}

@linhpha linhpha marked this pull request as ready for review August 7, 2023 22:27
@linhpha linhpha requested a review from ZacSweers August 7, 2023 22:27
@@ -129,7 +122,7 @@ internal class ApkVersioningPlugin : Plugin<Project> {
val mappedVersionCodeProvider =
versionCodeProvider.map { rawCode ->
@Suppress("MagicNumber")
ApkVersioning.VERSION_CODES.getValue(variantOutput.abiString) * 10000000 + rawCode
ApkVersioning.VERSION_CODE * 10000000 + rawCode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's drop this 10000000 and the VERSION_CODE magic constant and handle it internally in the slack repo's gradle.properties, no reason to impose this heh. Then we can just pass versionCodeProvider directly to the variantOutput.versionCode

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the calculation here 37159ee

Per this change, we'll need to update the version code in release.version to 90012557 right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup exactly

Copy link
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. One possibly tricky thing is that we either need to do a release with this cherry-picked onto a branch from the previous non-kotlin 1.9 update or do them all at once. What do you think?

@linhpha
Copy link
Collaborator Author

linhpha commented Aug 7, 2023

Lgtm. One possibly tricky thing is that we either need to do a release with this cherry-picked onto a branch from the previous non-kotlin 1.9 update or do them all at once. What do you think?

I think I'd prefer the latter cause it's easier 😅 I've been testing locally the upgrade with this change and it should work 🤞

@ZacSweers
Copy link
Collaborator

Kk I'll take care of it

@ZacSweers ZacSweers added this pull request to the merge queue Aug 8, 2023
Merged via the queue into main with commit 0501e32 Aug 8, 2023
2 checks passed
@ZacSweers ZacSweers deleted the lp_unify_version_code branch August 8, 2023 01:00
ZacSweers pushed a commit that referenced this pull request Aug 8, 2023
I forgot to update the default version code for local build in #503.
Before, devs usually get 30009999. This PR updates the version to be
90009999 to make it consistent with CI build number



<!--
  ⬆ Put your description above this! ⬆

  Please be descriptive and detailed.
  
Please read our [Contributing
Guidelines](https://github.com/tinyspeck/slack-gradle-plugin/blob/main/.github/CONTRIBUTING.md)
and [Code of Conduct](https://slackhq.github.io/code-of-conduct).

Don't worry about deleting this, it's not visible in the PR!
-->
ZacSweers pushed a commit that referenced this pull request Aug 8, 2023
We can move away from having separate version code for each build type
now that we're publishing app bundle

This change should also help resolve issues with AGP 8.1 upgrade



<!--
  ⬆ Put your description above this! ⬆

  Please be descriptive and detailed.
  
Please read our [Contributing
Guidelines](https://github.com/tinyspeck/slack-gradle-plugin/blob/main/.github/CONTRIBUTING.md)
and [Code of Conduct](https://slackhq.github.io/code-of-conduct).

Don't worry about deleting this, it's not visible in the PR!
-->
ZacSweers pushed a commit that referenced this pull request Aug 8, 2023
I forgot to update the default version code for local build in #503.
Before, devs usually get 30009999. This PR updates the version to be
90009999 to make it consistent with CI build number



<!--
  ⬆ Put your description above this! ⬆

  Please be descriptive and detailed.
  
Please read our [Contributing
Guidelines](https://github.com/tinyspeck/slack-gradle-plugin/blob/main/.github/CONTRIBUTING.md)
and [Code of Conduct](https://slackhq.github.io/code-of-conduct).

Don't worry about deleting this, it's not visible in the PR!
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants