Remove legacy localisation script references and actions #447
Conversation
We now have newer, better actions to achieve the same results: - `ios_generate_strings_file_from_code` - `ios_extract_keys_from_strings_files` - `ios_download_strings_files_from_glotpress` - `ios_merge_strings_files`
iangmaia
changed the title
There was a problem hiding this comment.
The diff looks good to me, and I checked the various call sites of the helper methods that you deleted and confirm that they were not used elsewhere outside of the actions you also removed. Great cleanup!! 🧹 🧹 🧹
Dropped an idea about starting a quick migration guide for all the things we're breaking in those various cleanups you're doing. But isn't a blocker for this to land.
| files_list.append File.join('fastlane', 'Deliverfile') if include_deliverfile | ||
| if include_metadata | ||
| files_list.append File.join('fastlane', 'download_metadata.swift') | ||
| files_list.append File.join(ENV['PROJECT_ROOT_FOLDER'], ENV['PROJECT_NAME'], 'Resources', ENV['APP_STORE_STRINGS_FILE_NAME']) |
There was a problem hiding this comment.
Seeing this, I wonder if it would be worth it to document somewhere the changes potentially needed in client projects when migrating to the next major version that will contain this change.
We don't really have a "Migration Guides" documentation so far in release-toolkit (because most of our past breaking changes were usually things we already adopted in all client projects before getting rid of them in the toolkit). But for all these changes we're making here, given how it can be easy to lose track, maybe that would be worth it to make a list of things to check when migrating? (Not sure where, maybe in a docs/MigrationGuide.md file for example?)
Typical examples of things we could remind people to check on their client projects when migrating it to the future new toolkit:
- Ensure that any call to
ios_bump_version_releasein their Fastfile already passedskip_glotpress: true. If they passedfalseor didn't provide a value (falsebeing the default for thisConfigItem), that means they'll have to ensure (1) they don't usedownload_metadata.swiftanymore, or if not, it'd be high time for them to migrate to the new tooling instead [link to my Project Thread maybe], and (2) that they don't rely on thisios_bump_version_releasefor commiting the.po/.potfile - Note that they can delete the
ENV['APP_STORE_STRINGS_FILE_NAME']from theirFastfile, if they still have it defined, because it's not used by anything anymore.
Mentioning all this because, while reviewing your PR, I wanted to check the status of WordPress-iOS to validate this wouldn't break it, and while I confirmed that the only place where it calls ios_bump_version_release it also passes skip_glotpress: true—so we're good there—I also noticed that we kept the declaration of that now-unused env var while it is not needed anymore and would make for a good occasion of a quick cleanup.
There was a problem hiding this comment.
Seeing this, I wonder if it would be worth it to document somewhere the changes potentially needed in client projects when migrating to the next major version that will contain this change.
This is a nice idea! It's a good point on cases where there isn't necessarily a breaking change but still the client projects could do a clean up on unnecessary things they previously had to do, or to implement a different behaviour than they'd previously have.
I think the examples you gave for this PR sound like a good starting point.
What about a MIGRATION.md file in the root, next to the CHANGELOG.md?
The way I see it, the structure and the process would be very similar to the change log, with sections for each version containing each a bullet list of items that require attention during the migration from the previous version. Then all PRs making such changes would include something in the list.
What do you think? I can kick it off for this PR, it shouldn't take long :-)
Co-authored-by: Olivier Halligon <[email protected]>
|
I've created and pushed a new branch |
What does it do?
This is a clean up removing the remainder references to the legacy localisation tooling, following up #443.
It contains:
ios_localize_projectandios_update_metadata.localize_project,update_metadataandstrings_filesonIos::GitHelperinclude_metadatafrom the methodIos::GitHelper.commit_version_bump(include_deliverfile:)Related PRs
skip_glotpressand related cleanup #443, the PR this one was based on, has been merged.Deliverfile.Checklist before requesting a review
bundle exec rubocopto test for code style violations and recommendationsspecs/*_spec.rb) if applicablebundle exec rspecto run the whole test suite and ensure all your tests passCHANGELOG.mdfile to describe your changes under the approprioate existing###subsection of the existing## Trunksection.