Jetpack App: Update release toolkit to support Jetpack #258

JavonDavis · 2021-05-12T23:37:56Z

This PR introduces changes that were primarily needed to support the new Jetpack app in the WPAndroid repo but are more focused on updating how versions are managed in our current and future Android apps. The proposals here are:

The use of a new app parameter/environment variable to add an additional context when doing versioning. This is a new parameter that can be passed as an option to the commands related to versioning, this value can can also be set as an environment variable for projects that don't have multiple 'apps' within them.
It also proposes the use of a new properties file in the project repos to manage versioning, a version.properties file that follows the following key-value format

appName.versionName=<version name>
appName.versionCode=<version code>

appName.alpha.versionName=<version name>
appName.alpha.versionCode=<version code>

Example here: https://github.com/wordpress-mobile/WordPress-Android/blob/fe4352dbdf2d8b56f760ceeb6a0bd591883b23e4/version.properties

With accompanying changes in the build.gradle file to read the versions from this file instead of being defined in the build.gradle. Something like so ->

def versionProperties = new Properties()
    file("../version.properties").withInputStream { versionProperties.load(it) }

    compileSdkVersion rootProject.compileSdkVersion

    defaultConfig {
	@@ -60,9 +63,10 @@ android {
        if (project.hasProperty("versionName")) {
            versionName project.property("versionName")
        } else {
            versionName versionProperties.getProperty("wordpress.alpha.versionName")
        }

        versionCode versionProperties.getProperty("wordpress.alpha.versionCode").toInteger()

This PR also proposes moving the logic for updating the version into a gradle task which can be a bit more stable than our previous approach, see proposed gradle task currently used in Jetpack App: Update infrastructure to support Jetpack app WordPress-Android#14613... The PR assumes this task exists and relies on it to update the version.properties file

task "updateVersionProperties" {
    doLast {
        ant.propertyfile(
                file: "../version.properties") {
            entry( key: "$key", value: "$value")
        }
    }
}

…rameter

codecov-commenter · 2021-05-12T23:44:08Z

Codecov Report

Merging #258 (d10336d) into develop (3417301) will decrease coverage by 0.14%.
The diff coverage is 21.26%.

@@             Coverage Diff             @@
##           develop     #258      +/-   ##
===========================================
- Coverage    51.66%   51.51%   -0.15%     
===========================================
  Files          108      108              
  Lines         4477     4523      +46     
===========================================
+ Hits          2313     2330      +17     
- Misses        2164     2193      +29

Impacted Files	Coverage Δ
...kit/actions/android/android_betabuild_prechecks.rb	`25.00% <0.00%> (-0.54%)`	⬇️
...toolkit/actions/android/android_build_prechecks.rb	`34.37% <0.00%> (-2.30%)`	⬇️
...olkit/actions/android/android_bump_version_beta.rb	`31.42% <0.00%> (-5.42%)`	⬇️
...kit/actions/android/android_bump_version_hotfix.rb	`29.03% <0.00%> (-5.26%)`	⬇️
...it/actions/android/android_bump_version_release.rb	`26.19% <0.00%> (-3.81%)`	⬇️
...it/actions/android/android_codefreeze_prechecks.rb	`34.21% <0.00%> (-1.91%)`	⬇️
...ns/android/android_completecodefreeze_prechecks.rb	`38.70% <0.00%> (-1.30%)`	⬇️
...ctions/android/android_current_branch_is_hotfix.rb	`59.09% <0.00%> (-5.91%)`	⬇️
...lkit/actions/android/android_finalize_prechecks.rb	`41.93% <0.00%> (-1.40%)`	⬇️
...olkit/actions/android/android_get_alpha_version.rb	`55.00% <0.00%> (-6.12%)`	⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3417301...d10336d. Read the comment docs.

AliSoftware

I like the suggestion to introduce a version.properties 👍
But that also means that this PR is quite a breaking change, as it will require the apps to update:

Their Fastfile to update all the calls to the various actions you modified, to provide the app name now (especially WooCommerce Android as you otherwise made the app name default to wordpress).
Their build.gradle file to load the new version.properties and make the versionName/versionCode references point to it.
Create their version.properties.

That is quite the breaking change, so this should not be merged before we do an intermediate release of the release-toolkit before this PR, which first contains all the pending but non-breaking fixes that are waiting for review, and only could we merge this PR and immediately create a new major version (since breaking changes)

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_betabuild_prechecks.rb

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_build_prechecks.rb

lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_git_helper.rb

lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_version_helper.rb

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_betabuild_prechecks.rb

…xplicitly

CHANGELOG.md

AliSoftware

Still spotted an inverted logic remaining…

Once this is fixed and the other nits are addressed, should hopefully be finally good to merge tho. (Don't forget to also create the issue about alpha env var removal to handle it later)

AliSoftware · 2021-06-16T17:14:43Z

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_bump_version_beta.rb

@@ -23,14 +34,21 @@ def self.run(params)
      #####################################################

      def self.description
-        'Bumps the version of the app for a new beta'
+        'Bumps the version of the app for a new beta. Depends on a gradle task to update the keys in a version.properties file.'


We need to think about the people who will adopt this version of the release toolkit and have never seen your PR nor code. They should not have to guess what gradle task they need to add to make this working.

Suggested change

'Bumps the version of the app for a new beta. Depends on a gradle task to update the keys in a version.properties file.'

'Bumps the version of the app for a new beta. Requires the `updateVersionProperties` gradle task to update the keys if you are using a `version.properties` file.'

(And same suggestion (search/replace all) on all the places and other actions where you mentioned this)

That's true! Agree with being more explicit here

AliSoftware · 2021-06-16T17:18:00Z

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_bump_version_final_release.rb

+          FastlaneCore::ConfigItem.new(key: :app,
+                                       env_name: 'PROJECT_NAME',
+                                       description: 'The name of the app to get the release version for',
+                                       is_string: true), # true: verifies the input is a string, false: every kind of value


Nit: This is legacy api of ConfigItem constructor. The more modern version is:

Suggested change

is_string: true), # true: verifies the input is a string, false: every kind of value

type: String),

This is not a blocker and can be fixed in a subsequent PR; but could be nice to take the occasion to fix while you're at it; this suggestion applies in every places and actions where you added this of course.

Ahhh I think I'd rather tackle it in another PR, opened #278 to track it.

AliSoftware · 2021-06-16T17:22:35Z

lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_git_helper.rb

+          if Fastlane::Helper::Android::VersionHelper.properties_file_exists
+            Fastlane::Helper::GitHelper.commit(
+              message: 'Bump version number',
+              files: File.join(ENV['PROJECT_ROOT_FOLDER'], ENV['PROJECT_NAME'], 'build.gradle'),
+              push: true
+            )
+          else
+            Fastlane::Helper::GitHelper.commit(
+              message: 'Bump version number',
+              files: File.join(ENV['PROJECT_ROOT_FOLDER'], 'version.properties'),
+              push: true
+            )
+          end


🚨 This one still seems inverted, commiting build.gradle if version.properties exists…

AliSoftware · 2021-06-16T17:24:41Z

CHANGELOG.md

+* Support for a version.properties to manage app versioning - all existing paths remain intact and new paths are only used when a version.properties file is present
+* Add support for providing an `app:` parameter to most versioning-related actions to allow support for multiple apps hosted in a monorepo
+* Supporting the new version.properties file also allows for the HAS_ALPHA_VERSION variable to be removed as the alpha reference in the properties file will be used going forward.
+* Clients adopting the new version.properties will need to implement a gradle task to update the version.properties file named `updateVersionProperties`


💄 nit:

Suggested change

* Clients adopting the new version.properties will need to implement a gradle task to update the version.properties file named `updateVersionProperties`

* Clients adopting the new `version.properties` will need to implement a gradle task named `updateVersionProperties` to update the `version.properties` file.

AliSoftware · 2021-06-16T17:25:05Z

CHANGELOG.md

-_None_
+* Support for a version.properties to manage app versioning - all existing paths remain intact and new paths are only used when a version.properties file is present
+* Add support for providing an `app:` parameter to most versioning-related actions to allow support for multiple apps hosted in a monorepo
+* Supporting the new version.properties file also allows for the HAS_ALPHA_VERSION variable to be removed as the alpha reference in the properties file will be used going forward.


💄 small nit:

Suggested change

* Supporting the new version.properties file also allows for the HAS_ALPHA_VERSION variable to be removed as the alpha reference in the properties file will be used going forward.

* Supporting the new `version.properties` file also allows for the `HAS_ALPHA_VERSION` variable to be removed as the alpha reference in the properties file will be used going forward.

AliSoftware · 2021-06-16T17:25:26Z

CHANGELOG.md

 ### New Features

-_None_
+* Support for a version.properties to manage app versioning - all existing paths remain intact and new paths are only used when a version.properties file is present


💄 small nitpick:

Suggested change

* Support for a version.properties to manage app versioning - all existing paths remain intact and new paths are only used when a version.properties file is present

* Support for a `version.properties` to manage app versioning - all existing paths remain intact and new paths are only used when a `version.properties` file is present.

AliSoftware · 2021-06-16T17:28:35Z

Also there's a CI failure on tests, we should fix it before merging, in order to make CI actually run the tests and ensure those changes didn't break any existing test nor introduce regressions.

oguzkocer · 2021-06-16T17:46:30Z

@JavonDavis About your review request, since @AliSoftware is already reviewing this, and he's our expert :) I don't know if I can add any value to this PR by reviewing it. I have quite a few things to get through in the next couple days, so I wanted to check if you'd still like me to review it or not. If you do, I'll try to get to it before my EOD.

JavonDavis · 2021-06-17T03:02:41Z

If you do, I'll try to get to it before my EOD.

No worries @oguzkocer, though I'd definitely appreciate your feedback on the changes on the Android side once I open it for review for this so feel free to ignore this one!

AliSoftware

Ok, all looks good now 🎉

AliSoftware · 2021-06-21T08:58:14Z

lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_version_helper.rb

@@ -46,8 +46,8 @@ def self.get_public_version(app)
        #
        # @return [Hash] A hash with 2 keys "name" and "code" containing the extracted version name and code, respectively
        #
-        def self.get_release_version(app)
-          return get_version_from_properties(product_name: app) if properties_file_exists
+        def self.get_release_version(product_name:)


You should update the comment on line 45 too, to match the param name 😉

# @param [String] product_name The name of the app to be used for beta and alpha version update (i.e the prefix in version.properties file)

JavonDavis added 7 commits May 7, 2021 11:32

abstract section name when getting release version for android

0a111bc

add param to get release version to allow for handling jetpack

5fceded

update syntax errror

15dd87e

include app in available options

c88cc24

update action callsites to now include an app name

1cc4399

update actions that were missed to include app param

31d91fb

remove HAS_ALPHA env variable and fix inconsistencies with new app pa…

697bcf6

…rameter

JavonDavis added the enhancement New feature or request label May 12, 2021

JavonDavis self-assigned this May 12, 2021

revert gemfile changes

679b33b

JavonDavis added 12 commits May 12, 2021 19:30

clean up remaining env var uses

9f991bb

address danger issues

67e007b

address danger issues

fb99670

address danger issues

89194c3

allow for the use of relase flavor env var

4135aa9

change env var name

b95e5e2

update naming and comments to be more consistent

e587c66

add tests around new version properties logic

17788b0

address Danger issues

508ee83

address Danger issues

3129d01

address Danger issues

fd19dca

address Danger issues

d1e095e

JavonDavis changed the title ~~Update/jetpack~~ Jetpack App: Update release toolkit to support Jetpack May 13, 2021

JavonDavis marked this pull request as ready for review May 13, 2021 18:27

JavonDavis requested a review from a team May 13, 2021 18:27

AliSoftware reviewed May 17, 2021

View reviewed changes

lib/fastlane/plugin/wpmreleasetoolkit/actions/android/android_betabuild_prechecks.rb Outdated Show resolved Hide resolved

AliSoftware requested a review from a team May 17, 2021 17:41

zwarm mentioned this pull request May 26, 2021

Jetpack App: Update Screenshot Infra (fastlane) wordpress-mobile/WordPress-Android#14724

Closed

3 tasks

JavonDavis added 2 commits June 15, 2021 10:08

update Changelog

3987111

address existing instances of app variable assignment using env var e…

fb76ece

…xplicitly

AliSoftware reviewed Jun 15, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

update Changelog

2e3a8e2

JavonDavis requested review from AliSoftware and a team June 15, 2021 15:24

update Changelog

bf14099

AliSoftware reviewed Jun 15, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

AliSoftware requested review from AliSoftware and jkmassel June 15, 2021 16:34

JavonDavis requested a review from oguzkocer June 15, 2021 16:41

AliSoftware requested changes Jun 16, 2021

View reviewed changes

JavonDavis added 2 commits June 16, 2021 22:13

update details in bump actions to include name of gradle task

1e7c77a

fix inverted logic in commit helper

841e8da

JavonDavis mentioned this pull request Jun 17, 2021

Remove the need for the alpha version env var #277

Closed

JavonDavis added 3 commits June 16, 2021 22:18

update changelog

3496330

update changelog

354de4e

address shallow clone issue on ci

b58fc23

JavonDavis requested review from AliSoftware and removed request for oguzkocer June 17, 2021 03:02

AliSoftware approved these changes Jun 17, 2021

View reviewed changes

update get_release version callsites

b695e16

This was referenced Jun 18, 2021

Merge release/jetpack/17.6 into jetpack_ci_lanes after Jetpack 17.5-rc-2 wordpress-mobile/WordPress-Android#14870

Merged

Jetpack App: Update infrastructure to support Jetpack app wordpress-mobile/WordPress-Android#14613

Merged

AliSoftware reviewed Jun 21, 2021

View reviewed changes

JavonDavis merged commit c0df075 into develop Jun 22, 2021

JavonDavis deleted the update/jetpack branch June 22, 2021 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack App: Update release toolkit to support Jetpack #258

Jetpack App: Update release toolkit to support Jetpack #258

JavonDavis commented May 12, 2021 •

edited

Loading

codecov-commenter commented May 12, 2021 •

edited

Loading

AliSoftware left a comment

AliSoftware left a comment

AliSoftware Jun 16, 2021 •

edited

Loading

JavonDavis Jun 17, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

JavonDavis Jun 17, 2021

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware commented Jun 16, 2021

oguzkocer commented Jun 16, 2021

JavonDavis commented Jun 17, 2021

AliSoftware left a comment

AliSoftware Jun 21, 2021

	'Bumps the version of the app for a new beta. Depends on a gradle task to update the keys in a version.properties file.'
	'Bumps the version of the app for a new beta. Requires the `updateVersionProperties` gradle task to update the keys if you are using a `version.properties` file.'

	is_string: true), # true: verifies the input is a string, false: every kind of value
	type: String),

	* Clients adopting the new version.properties will need to implement a gradle task to update the version.properties file named `updateVersionProperties`
	* Clients adopting the new `version.properties` will need to implement a gradle task named `updateVersionProperties` to update the `version.properties` file.

	* Supporting the new version.properties file also allows for the HAS_ALPHA_VERSION variable to be removed as the alpha reference in the properties file will be used going forward.
	* Supporting the new `version.properties` file also allows for the `HAS_ALPHA_VERSION` variable to be removed as the alpha reference in the properties file will be used going forward.

	* Support for a version.properties to manage app versioning - all existing paths remain intact and new paths are only used when a version.properties file is present
	* Support for a `version.properties` to manage app versioning - all existing paths remain intact and new paths are only used when a `version.properties` file is present.

Jetpack App: Update release toolkit to support Jetpack #258

Jetpack App: Update release toolkit to support Jetpack #258

Conversation

JavonDavis commented May 12, 2021 • edited Loading

codecov-commenter commented May 12, 2021 • edited Loading

Codecov Report

AliSoftware left a comment

Choose a reason for hiding this comment

AliSoftware left a comment

Choose a reason for hiding this comment

AliSoftware Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

JavonDavis Jun 17, 2021 • edited Loading

Choose a reason for hiding this comment

AliSoftware Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

JavonDavis Jun 17, 2021

Choose a reason for hiding this comment

AliSoftware Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

AliSoftware Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

AliSoftware Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

AliSoftware Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

AliSoftware commented Jun 16, 2021

oguzkocer commented Jun 16, 2021

JavonDavis commented Jun 17, 2021

AliSoftware left a comment

Choose a reason for hiding this comment

AliSoftware Jun 21, 2021

Choose a reason for hiding this comment

JavonDavis commented May 12, 2021 •

edited

Loading

codecov-commenter commented May 12, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

JavonDavis Jun 17, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading

AliSoftware Jun 16, 2021 •

edited

Loading