ReactiveCircus / app-versioning

A Gradle Plugin for lazily generating Android app's versionCode & versionName from Git tags.
Apache License 2.0
205 stars 3 forks source link

Request: Document what to do with versionCode and versionName in android defaultConfig #1

Closed TylerMcCraw closed 4 years ago

TylerMcCraw commented 4 years ago

As someone who is new to this plugin, it would be helpful to understand what should be done with the existing versionCode and versionName in the main module's android.defaultConfig config. If they are ignored by this plugin, then it might be worth mentioning in the README that you can leave them alone because it won't affect the final version code and name.

i.e. what does someone do with this config:

android {
   defaultConfig {
      versionCode 1
      versionName 1.0.0
   } 
}
ychescale9 commented 4 years ago

Thanks! The docs are very much in progress but this reminds me to test this behavior and maybe add some functional tests. Another thing I havenโ€™t figured out is the fallback mechanism eg do we force them to have git tags? Do we relax the SemVer format in those tags etc and how to fallback to the versionCode / versionName user defines in defaultConfig if this plugin has nothing to contribute.

TylerMcCraw commented 4 years ago

Great questions.

  1. I personally think that requiring git tags for now is best. It means you're being more prescriptive based on best practices, and I don't see any harm in that since this plugin needs something to rely on that is stable.
  2. I would allow for relaxing semantic versioning format checks down to the minor version (i.e. require that there must at least be a major and minor version, but the patch can be allowed to not exist). This would be a nice flexibility for companies who sometimes do not include a patch version. Though, I don't think this is necessary for a 1.0 ๐Ÿ˜ release of your plugin.
  3. defaultConfig is the best fallback option for sure. I assume the plugin task would just quit running if it finds that no Git tags exist (maybe it throws a warning to output to say "No Git tags exist, so a new app version was not generated. Falling back to defaultConfig...")
ychescale9 commented 4 years ago

I personally think that requiring git tags for now is best. It means you're being more prescriptive based on best practices, and I don't see any harm in that since this plugin needs something to rely on that is stable.

I generally agree:) A couple of edge cases I can think of where requiring git tag might be inconvenient:

  1. In a fresh project where there hasn't been a release yet - in this case the plugin should probably not fail the build (forcing them to not include the plugin until they have a least 1 tag) - currently there's a requireValidGitTag config which when sets to false (default) falls back to "0.0.0" and 0 if no valid git tag can be found, of fails the build if it's set to true. What's a valid git-tag is also not well-defined, as there might be some non-SemVer compliant tags in the branch or some random tags you just want to ignore... So I'm thinking about changing this to something like requiresSemVerTag - so you either use strictly SemVer compliant tags in your repo, or you don't but you can use the overrideVersionName and overrideVersionCode lambdas to define your own rules
  2. I know some projects would not associate the versionCode with a git tag, but instead just use "the number of commits in the current branch" e.g. the output of git rev-list --count HEAD. In this case we still want to allow them to use the overrideVersionCode lambda but the current GitTag will need to be changed to GitInfo or something with more Git related information.

I would allow for relaxing semantic versioning format checks down to the minor version (i.e. require that there must at least be a major and minor version, but the patch can be allowed to not exist). This would be a nice flexibility for companies who sometimes do not include a patch version. Though, I don't think this is necessary for a 1.0 ๐Ÿ˜ release of your plugin.

Yeah this is tricky. Being opinionated is great but limits the potential user base as there'll always be projects that do something slightly non-standard... As mentioned earlier I think it's easier to just strictly check for SemVer - the default is combining the 3 digits of semver to calculate the versionCode using 10000 MAJOR + 100 MINOR + PATCH as the formula; if you don't agree with this 100% it's easy to customize these with the overrideVersionCode lambda where you have access to the GitTag and the Providers to help you get a system property or envar lazily.

defaultConfig is the best fallback option for sure. I assume the plugin task would just quit running if it finds that no Git tags exist (maybe it throws a warning to output to say "No Git tags exist, so a new app version was not generated. Falling back to defaultConfig...")

I haven't looked into how to hook into the AGP APIs to implement this fallback so currently it just falls back to "0.0.0", 0 which is not ideal. But we'll figure something out.

Thanks for the great feedback! I might move my TODOs / roadmap to GitHub issues so it's more transparent and easier for this kind of discussions :)

ychescale9 commented 4 years ago

I've just released 0.3.0 with some changes around how some of these edge cases are handled:

Please have a try and let me know what you think ๐Ÿ˜ƒ

TylerMcCraw commented 4 years ago

I've tested these changes w/ v0.4.0 and they look great. I also see all of this info is in the README now, so feel free to close the issue.