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

Merge final release/20.9 #17340

Merged
merged 5 commits into from
Oct 14, 2022
Merged

Merge final release/20.9 #17340

merged 5 commits into from
Oct 14, 2022

Conversation

AliSoftware
Copy link
Contributor

  • changelog/*.txt files updated with latest translations
  • values*/strings.xml updated with latest translations
  • Version bumped in version.properties
  • WPAndroid submitted to Google for review
  • JPAndroid submitted to Google for review

@AliSoftware AliSoftware added the Releases Label related to managing releases label Oct 14, 2022
@AliSoftware AliSoftware added this to the 21.0 milestone Oct 14, 2022
@AliSoftware AliSoftware requested a review from a team October 14, 2022 13:52
@AliSoftware AliSoftware self-assigned this Oct 14, 2022
@ParaskP7 ParaskP7 self-assigned this Oct 14, 2022
@AliSoftware AliSoftware enabled auto-merge October 14, 2022 13:55
@ParaskP7
Copy link
Contributor

👋 @AliSoftware !

If I am not mistaken, the new fastlane/metadata/android/en-US/changelogs/1281.txt file is missing, can you confirm that? 🤔

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Oct 14, 2022

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17340-c2bf8ce.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commitc2bf8ce
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Contributor

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17340-de70a62.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commitde70a62
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@AliSoftware
Copy link
Contributor Author

👋 @AliSoftware !

If I am not mistaken, the new fastlane/metadata/android/en-US/changelogs/1281.txt file is missing, can you confirm that? 🤔

What an eagle eye! Thanks so much for noticing that, you're a life-saver!

I also checked at what I submitted to Play Store, and indeed the <en-US>…</en-US> entry of the release notes were empty as a result, so indeed we almost submitted a release with no English release notes (and with the default ones falling back to the Arabic ones instead, apparently)!

image image

The root cause of this bug is likely due to the same issue that what happened in a 20.6 too (see pdnsEh-jJ-p2#comment-1233), where the en-GB translation were missing for the release notes, and since we currently do something very ugly with mapping the source copy of the en-GB locale to retrieve the en-US's changelogs/*.txt, and if there is no en-GB entry in GlotPress for a release… then the entire entry is absent from the GlotPress download, hence why we fail to extract the associated source file from it.

We should definitively remove that en-GB => en-US hacky mapping at some point, and instead copy the files from the WordPress/metadata/*.txt source of truth like we do for the case of Jetpack. I'll try to address that in a dedicated PR on trunk.

Note: Ideally in the future we could even consider having fastlane/*metadata/en-US (or even fastlane/*metadata/default) be the source of truth, instead of it being WordPress/*metadata and then having to copy that over to fastlane/*metadata/en-US during final release, that would simplify things greatly. The case for release_notes.txt is a bit special though, as at some point during the release that file contains transient content (the release notes extracted from the top section of RELEASE-NOTES.txt, before they get editorialized), so that wouldn't solve us keeping to have a separate, transient source of truth for that one I guess?

Because of a bug in our tooling, the `fastlane/metadata/android/en-US/changelogs/1281.txt` file was not generated by `gp_downloadmetadata` because GlotPress didn't have a translation for `en-GB` locale for it (and in `gp_downloadmetadata`, we extract the original `en-US` from the export of `en-GB`... but if that entry is non-existant to begin with, that hack fails anyway).

See pdnsEh-jJ-p2 for details of this tooling bug

Thanks @ParaskP7 for noticing the missing changelog in the PR in the first place in #17340 (comment) !!
@AliSoftware
Copy link
Contributor Author

AliSoftware commented Oct 14, 2022

👋 @AliSoftware !

If I am not mistaken, the new fastlane/metadata/android/en-US/changelogs/1281.txt file is missing, can you confirm that? 🤔

Fixed in c2bf8ce, and manually updated the Release Notes in Play Store Console accordingly as well, which now defaults back to en-US 🎉

image

@ParaskP7
Copy link
Contributor

ParaskP7 commented Oct 14, 2022

What an eagle eye! Thanks so much for noticing that, you're a life-saver!

🦅 👀

I also checked at what I submitted to Play Store, and indeed the <en-US>…</en-US> entry of the release notes were empty as a result, so indeed we almost submitted a release with no English release notes!

Good thinking, nice catch! 🥇

(and with the default ones falling back to the Arabic ones instead, apparently)

😵‍💫

The root cause of this bug is likely due to the same issue that what happened in a 20.6 too (see pdnsEh-jJ-p2#comment-1233), where the en-GB translation were missing for the release notes, and since we currently do something very ugly with mapping the source copy of the en-GB locale to retrieve the en-US's changelogs/*.txt, and if there is no en-GB entry in GlotPress for a release… then the entire entry is absent from the GlotPress download, hence why we fail to extract the associated source file from it.

👀

We should definitively remove that en-GB => en-US hacky mapping at some point, and instead copy the files from the WordPress/metadata/*.txt source of truth like we do for the case of Jetpack. I'll try to address that in a dedicated PR on trunk.

👍 💯

Note: Ideally in the future we could even consider having fastlane/*metadata/en-US (or even fastlane/*metadata/default) be the source of truth, instead of it being WordPress/*metadata and then having to copy that over to fastlane/*metadata/en-US during final release, that would simplify things greatly. The case for release_notes.txt is a bit special though, as at some point during the release that file contains transient content (the release notes extracted from the top section of RELEASE-NOTES.txt, before they get editorialized), so that wouldn't solve us keeping to have a separate, transient source of truth for that one I guess?

Thanks for this note! 🙇

Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

LGTM! 💯

Fixed in c2bf8ce, and manually updated the Release Notes in Play Store Console accordingly as well, which now defaults back to en-US 🎉

Awesome, thanks for the double fix @AliSoftware ! 🥇 ❤️ 🚀

@AliSoftware
Copy link
Contributor Author

We should definitively remove that en-GB => en-US hacky mapping at some point, and instead copy the files from the WordPress/metadata/*.txt source of truth like we do for the case of Jetpack. I'll try to address that in a dedicated PR on trunk.

Follow-up on this: #17341

@AliSoftware AliSoftware mentioned this pull request Oct 28, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Releases Label related to managing releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants