-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Merge 17.8-rc-3 to develop #15042
Merge 17.8-rc-3 to develop #15042
Conversation
Co-authored-by: Olivier Halligon <[email protected]>
…adata [Jetpack] Update playstore metadata
…elease_1.57.1 Integrate gutenberg-mobile release 1.57.1
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
You can test the changes on this Pull Request by downloading the APKs: |
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.
-
versionName
rc
number andversionCode
bumps inbuild.gradle
|version.properties
-
string.xml
updates - Diffs from the PRs that made it into this beta
It might be worth for the reviewer to resolve the conflict in a temporary branch to make sure I got it right.
I did that and came to the same resolution. I did see the trailing whitespace in my Vim, which removed it automatically on save (This is the way).
jetpack.versionName=17.8-rc-2 | ||
jetpack.versionCode=1076 | ||
jetpack.versionName=17.8-rc-3 | ||
jetpack.versionCode=1077 |
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.
Just checking it's okay to "recycle" this version code, which was previously used in WordPress 17.8-rc-2?
I think it is because this is the Jetpack config, which is a different app.
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 think you meant it's used in WordPress 17.8-rc-3
since the end result of this ^ change is 17.8-rc-3
. With that assumption in mind, I believe the answer is yes, because it's a different app. However, worth pinging @AliSoftware for when he gets back, just so he can double check the intent.
@AliSoftware I don't know why these versions were separated (although I reviewed PRs about it, I can't remember the reason), so it's worth thinking about using a single version & version code for Jetpack and WordPress apps if we are going to always keep them in sync.
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.
Yep:
- @mokagio it's different apps in PlayStore so it's ok to use the same versionCode indeed.
- @oguzkocer That's a leftover of Javon's changes where he split the versioning for WP and JP, which I intend to revert. Now we use a single version and version code for JP and WP, so yes, definitively, there are plans to unify them and using a single entry there for it instead of having to keep them separate but update them both to the same value.
I have yet to find some time to pick that project back up (needed to focus on my other hotfix lanes project before AFK, and I'm hoping to tackle that revert of Javon's changes in release toolkit in this Sprint or the next), hopefully once this is tackled things will be way cleaner there.
Contains gb-mobile hotfix from #15035 and Jetpack metadata improvements from #15005.
There were 2 merge conflicts:
develop
release/17.8
was selected for resolving the conflict.It might be worth for the reviewer to resolve the conflict in a temporary branch to make sure I got it right.