release-toolkit
release-toolkit copied to clipboard
Introduce Locale helper models
This PR introduces the Locales
model class in the release toolkit. Its intent is to work way more easily with locale codes in our Fastfiles
, by replacing huge constant lists like those 65 lines in WPAndroid's Fastfile
with way shorter and nicer lines like:
RELEASE_NOTES_LOCALES = Fastlane::Locales.mag16
This will return an Array<Locale>
where Locale
is a struct with 5 fields containing the locale codes for each space (glotpress, android, google_play, ios, app_store).
A Locale
can also be converted into a Hash
for retro-compatibility with the existing toolkit actions that expect such a Hash
(even though in the future we might want to support both a Hash
and a Locale
for those actions, but that's for a separate story)
This means we will be able to replace those WCAndroid 18 lines with just this – using .map(&:to_h)
to convert it from an Array<Locale>
to an Array<Hash>
for compatibility with existing actions:
SUPPORTED_LOCALES = Fastlane::Locales[%w[ar de es fr he id it ja ko nl pt-br ru sv tr zh-cn zh-tw]].map(&:to_h)
Draft State
We still need to
- [ ] Complete the list of
ALL_KNOWN_LOCALES
: add the missing values forios
andapp_store
keys, then ensure all the codes are correct for all the various keys, and that we are not missing any known locale. - [x] Enable
from_ios
andfrom_app_store
family of tests once we have the values inALL_KNOWN_LOCALES
@mokagio I've added you as preliminary reviewer, feel free to take a first look (especially at the Locales
class API and at the tests) and provide initial feedback. I'll add the whole Owl team as final reviewers once ☝️ is completed and I undraft the PR.
1 Warning | |
---|---|
:warning: | Please add an entry in the CHANGELOG.md file to describe the changes made by this PR |
Generated by :no_entry_sign: Danger
Codecov Report
Merging #296 (61147cf) into develop (e02e57d) will increase coverage by
1.08%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## develop #296 +/- ##
===========================================
+ Coverage 51.64% 52.72% +1.08%
===========================================
Files 108 111 +3
Lines 4533 4637 +104
===========================================
+ Hits 2341 2445 +104
Misses 2192 2192
Impacted Files | Coverage Δ | |
---|---|---|
lib/fastlane/plugin/wpmreleasetoolkit.rb | 100.00% <100.00%> (ø) |
|
...astlane/plugin/wpmreleasetoolkit/models/locales.rb | 100.00% <100.00%> (ø) |
|
spec/locale_spec.rb | 100.00% <100.00%> (ø) |
|
spec/locales_spec.rb | 100.00% <100.00%> (ø) |
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 e02e57d...61147cf. Read the comment docs.
I'm actually unsure if we use/need list of locale codes in the iOS Fastfiles like we do in the Android ones 🤔 at least yet?
A quick look seem to indicate that we don't really rely on any hardcoded list for iOS, since when post-processing locales we just iterate over folder names in fastlane/metadata
, and the only other place where we need the list of locales is when we download them from GlotPress, for which, as of today, we still do it via the download_metadata.swift
Swift script, not via a ruby action where we could use that new Fastlane::Locales[…]
magic.
But I still think it would be nice to have those ios/app store locale codes in that list even if we don't yet really use them in our Fastfile, so that when we get the opportunity work on the "Improve Localization tooling" project (paaHJt-sd-p2) and likely migrate that Swift script into a fastlane action by then, we'd have those locale codes ready to be used.
Another point: unlike android's values-<code>
vs PlayStore locale codes being different, for iOS it seems (at least at first, from the couple ones I quickly checked) that the locale codes used in iOS (<code>.lproj
) and in AppStore are the same (some consistency at least, not like Android 😅 ), which means I'm unsure if there's a point keeping both :ios
and :app_store
keys in the Locale
struct (allows to get a nice mirroring with Android) vs only have the :ios
key (would break the symmetry, but avoids repetition). A middle ground would be to only use :ios
in the initializer and list of Struct fields, and declare alias_method :app_store, :ios
as an alias for ios
.
@mokagio Thoughts?
Sorry about closing the PR I went to click the text area but hit the close button 😮💨
Sorry for taking so long to reply!
But I still think it would be nice to have those ios/app store locale codes in that list even if we don't yet really use them in our Fastfile
+1 there's value in having them there for sure.
A middle ground would be to only use
:ios
in the initializer and list of Struct fields, and declarealias_method :app_store, :ios
as an alias forios
.
I like this idea very much.