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.

Sign up for GitHub

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

Jump to bottom

Remove legacy localisation script references and actions #447

Merged
merged 6 commits into from
Feb 10, 2023
Merged

Remove legacy localisation script references and actions #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3a29939
Remove legacy iOS localization actions and related helpers
mokagio Jun 2, 2022
0a769a7
Remove download_metadata.swift references
iangmaia Jan 24, 2023
69f5e03
Use get_from_env util method instead of accessing environment var dir…
iangmaia Jan 29, 2023
8961644
Update changelog
iangmaia Jan 29, 2023
c48d444
Add PR reference to changelog
iangmaia Jan 30, 2023
7aaa3b7
Update CHANGELOG.md
iangmaia Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Breaking Changes

- Remove the `skip_glotpress` parameter from the `ios_bump_version_release` action [#443]
- Remove the `ios_localize_project` and `ios_update_metadata` actions [#447]

### New Features

Expand Down
2 changes: 1 addition & 1 deletion lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_bump_version_beta.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def self.run(params)
Fastlane::Helper::Ios::VersionHelper.update_xc_configs(@new_beta_version, @short_version, @new_internal_version)
UI.message 'Done!'

Fastlane::Helper::Ios::GitHelper.commit_version_bump(include_deliverfile: false, include_metadata: false)
Fastlane::Helper::Ios::GitHelper.commit_version_bump(include_deliverfile: false)
end

#####################################################
Expand Down
2 changes: 1 addition & 1 deletion lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_bump_version_hotfix.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.run(params)
Fastlane::Helper::Ios::VersionHelper.update_xc_configs(@new_version, @new_short_version, @new_version_internal)
UI.message 'Done!'

Fastlane::Helper::Ios::GitHelper.commit_version_bump(include_deliverfile: update_deliverfile, include_metadata: false)
Fastlane::Helper::Ios::GitHelper.commit_version_bump(include_deliverfile: update_deliverfile)

UI.message 'Done.'
end
Expand Down
3 changes: 1 addition & 2 deletions lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_bump_version_release.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ def self.run(params)
UI.message 'Done!'

Fastlane::Helper::Ios::GitHelper.commit_version_bump(
include_deliverfile: !params[:skip_deliver],
include_metadata: false
include_deliverfile: !params[:skip_deliver]
)

UI.message 'Done.'
Expand Down
43 changes: 0 additions & 43 deletions lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_localize_project.rb
View file
Open in desktop

This file was deleted.

40 changes: 0 additions & 40 deletions lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_update_metadata.rb
View file
Open in desktop

This file was deleted.

53 changes: 2 additions & 51 deletions lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_git_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,67 +9,18 @@ module GitHelper
# This typically commits and pushes:
# - The files in `./config/*` – especially `Version.*.xcconfig` files
# - The `fastlane/Deliverfile` file (which contains the `app_version` line)
# - The `<ProjectRoot>/<ProjectName>/Resources/AppStoreStrings.pot` file, containing a key for that version's release notes
#
# @env PROJECT_ROOT_FOLDER The path to the git root of the project
# @env PROJECT_NAME The name of the directory containing the project code (especially containing the Resources/ subfolder)
#
# @param [Bool] include_deliverfile If true (the default), includes the `fastlane/Deliverfile` in files being commited
# @param [Bool] include_metadata If true (the default), includes the `fastlane/download_metadata.swift` file and the `.pot` file (which typically contains an entry or release notes for the new version)
#
def self.commit_version_bump(include_deliverfile: true, include_metadata: true)
files_list = [File.join(ENV['PROJECT_ROOT_FOLDER'], 'config', '.')]
def self.commit_version_bump(include_deliverfile: true)
files_list = [File.join(get_from_env!(key: 'PROJECT_ROOT_FOLDER'), 'config', '.')]
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'])
Copy link
Contributor

Choose a reason for hiding this comment

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

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_release in their Fastfile already passed skip_glotpress: true. If they passed false or didn't provide a value (false being the default for this ConfigItem), that means they'll have to ensure (1) they don't use download_metadata.swift anymore, 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 this ios_bump_version_release for commiting the .po/.pot file
  • Note that they can delete the ENV['APP_STORE_STRINGS_FILE_NAME'] from their Fastfile, 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.

Copy link
Contributor Author

@iangmaia iangmaia Feb 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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 :-)

end

Fastlane::Helper::GitHelper.commit(message: 'Bump version number', files: files_list, push: true)
end

# Calls the `Scripts/localize.py` script in the project root folder and push the `*.strings` files
#
# That script updates the `.strings` files with translations from GlotPress.
#
# @env PROJECT_ROOT_FOLDER The path to the git root of the project
# @env PROJECT_NAME The name of the directory containing the project code (especially containing the `build.gradle` file)
#
# @deprecated This method is only used by the `ios_localize_project` action, which is itself deprecated
# in favor of the new `ios_generate_strings_file_from_code` action
# @todo [Next Major] Remove this method once we fully remove `ios_localize_project`
#
def self.localize_project
Action.sh("cd #{get_from_env!(key: 'PROJECT_ROOT_FOLDER')} && ./Scripts/localize.py")

Fastlane::Helper::GitHelper.commit(message: 'Update strings for localization', files: strings_files, push: true) || UI.message('No new strings, skipping commit')
end

# Call the `Scripts/update-translations.rb` then the `fastlane/download_metadata` Scripts from the host project folder
#
# @env PROJECT_ROOT_FOLDER The path to the git root of the project
# @env PROJECT_NAME The name of the directory containing the project code (especially containing the `build.gradle` file)
#
# @todo Migrate the scripts, currently in each host repo and called by this method, to be helpers and actions
# in the release-toolkit instead, and move this code away from `ios_git_helper`.
#
def self.update_metadata
Action.sh("cd #{get_from_env!(key: 'PROJECT_ROOT_FOLDER')} && ./Scripts/update-translations.rb")

Fastlane::Helper::GitHelper.commit(message: 'Update translations', files: strings_files, push: false)

Action.sh('cd fastlane && ./download_metadata.swift')

Fastlane::Helper::GitHelper.commit(message: 'Update metadata translations', files: './fastlane/metadata/', push: true)
end

def self.strings_files
project_root = get_from_env!(key: 'PROJECT_ROOT_FOLDER')
project_name = get_from_env!(key: 'PROJECT_NAME')

Dir.glob(File.join(project_root, project_name, '**', '*.strings'))
end

def self.get_from_env!(key:)
ENV.fetch(key) do
UI.user_error! "Could not find value for \"#{key}\" in environment."
Expand Down
4 changes: 2 additions & 2 deletions spec/ios_bump_version_release_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
skip_deliver = false

expect(Fastlane::Helper::Ios::VersionHelper).to receive(:update_fastlane_deliver).with(next_version_short)
expect(Fastlane::Helper::Ios::GitHelper).to receive(:commit_version_bump).with(include_deliverfile: !skip_deliver, include_metadata: false)
expect(Fastlane::Helper::Ios::GitHelper).to receive(:commit_version_bump).with(include_deliverfile: !skip_deliver)

run_described_fastlane_action(
skip_deliver: skip_deliver,
Expand All @@ -35,7 +35,7 @@
skip_deliver = true

expect(Fastlane::Helper::Ios::VersionHelper).not_to receive(:update_fastlane_deliver)
expect(Fastlane::Helper::Ios::GitHelper).to receive(:commit_version_bump).with(include_deliverfile: !skip_deliver, include_metadata: false)
expect(Fastlane::Helper::Ios::GitHelper).to receive(:commit_version_bump).with(include_deliverfile: !skip_deliver)

run_described_fastlane_action(
skip_deliver: skip_deliver,
Expand Down