-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 23.6 code freeze #21911
Merge 23.6 code freeze #21911
Conversation
The release toolkit code depends on an environment variable that is no longer present. See https://github.com/wordpress-mobile/release-toolkit/blob/32e419afa781cf86ca70276f4386671b7fac0784/lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_git_helper.rb#L14-L18 and b1d4142 which removed the env var. Good riddance. Using a first-party library to DRY the way we commit version files might have been taking DRY on the wrong direction
Otherwise, `git commit -m`, which Fastlane uses under the hood, would fail.
Fastlane can do that for us under the hood.
Most of the time, the release/public and internal versions (`VERSION_LONG`) values are the same. But that's not guaranteed to be the same. Case in point, during code freeze they are momentarily out of sync. If we use the public file to read the internal version during code freeze, we'll end up with the build code set for the following version. Notice the difference between the internal build code in the header and the one that's written on disk (last line): ``` [19:13:18]: Cruising back to lane 'ios code_freeze' 🚘 [19:13:18]: Code Freeze: • New release branch from trunk: release/23.6 • Current release version and build code: 23.5 (23.5.0.3). • New release version and build code: 23.6 (23.6.0.0). • Current internal release version and build code: 23.5 (23.5.0.20231027) • New internal release version and build code: 23.6 (23.6.0.20231030) [19:13:18]: Do you want to continue? (y/n) y [19:13:18]: Creating release branch... [19:13:18]: $ git branch --list release/23.6 [19:13:19]: $ git checkout trunk [19:13:19]: ▸ Already on 'trunk' [19:13:19]: ▸ Your branch is ahead of 'origin/trunk' by 5 commits. [19:13:19]: ▸ (use "git push" to publish your local commits) [19:13:19]: $ git checkout -b release/23.6 [19:13:19]: ▸ Switched to a new branch 'release/23.6' [19:13:19]: ------------------------ [19:13:19]: --- Step: git_branch --- [19:13:19]: ------------------------ [19:13:19]: Done! New release branch is: release/23.6 [19:13:19]: Bumping release version and build code... [19:13:19]: Done! New Release Version: 23.6. New Build Code: 23.6.0.0 [19:13:19]: Bumping internal release version and build code... [19:13:19]: Done! New Internal Release Version: 23.6. New Internal Build Code: 23.7.0.20231030 ```
`false` is the desired default value for the `bypass` parameter. I noticed this when I replied no to the prompt but it did not abort as I expected. See how the lane moved to `setbranchprotection` even though the input was `n` ``` [19:13:19]: Ready to push changes to remote to let the automation configure it on GitHub? (y/n) n [19:14:13]: Aborting code completion. See you later. [19:14:13]: --------------------------------- [19:14:13]: --- Step: setbranchprotection --- [19:14:13]: --------------------------------- ========================================== This action (setbranchprotection) is deprecated ``` However, `false` vs `nil` is not the cause of the defective behavior. That is due to the branch that's meant to abort only printing a message about it and not calling `next`.
@@ -101,7 +101,7 @@ class ReaderPostCardCell: UITableViewCell { | |||
static let likeButtonHint = NSLocalizedString("reader.post.button.like.accessibility.hint", | |||
value: "Likes the post.", | |||
comment: "Accessibility hint for the like button on the reader post card cell") | |||
static let likedButtonHint = NSLocalizedString("reader.post.button.like.accessibility.hint", | |||
static let likedButtonHint = NSLocalizedString("reader.post.button.liked.accessibility.hint", |
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.
Love that our tooling picks these up :)
Next step, fail the build in the PR that introduced them. wordpress-mobile/release-toolkit#446
def commit_version_bump | ||
git_commit( | ||
path: [PUBLIC_CONFIG_FILE, INTERNAL_CONFIG_FILE], | ||
message: 'Bump version number', | ||
allow_nothing_to_commit: false | ||
) | ||
end |
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.
See also wordpress-mobile/release-toolkit#525
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
Generated by 🚫 dangerJS |
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.
Looks good to me. 👍
A bit of extra work to get this one out the door, between the WordPressKit 8.9.1 patch wordpress-mobile/WordPressKit-iOS#639 and a few adjustment to the lanes after recent updates. Refer to individual commits comments.
Release checks
RELEASE-NOTE.txt
Localizable.strings
updatedrelease_notes.txt
updated with notes fromRELEASE-NOTE.txt
for current version.xcconfig