Remove deprecated, unused actions #337
Conversation
mokagio
force-pushed
the
remove-deprecated-actions
branch
from
b454157 to
45d20b7
Compare
mokagio
changed the title
android_merge_translators_strings and ios_merge_translators_strings actions
I also verified that this wasn't used in WordPress, WooCommerce, or Simplenote.
I verified that is not used in WordPress, WooCommerce, or Simplenote. Interestingly, neither is the suggested alternative, `ios_merge_strings_file`.
Currently only used in Simplenote Android, which is on pause. See https://github.com/Automattic/simplenote-android/blob/ca4c775cf62dd0f3178e50564d8d4a17d17408ef/fastlane/Fastfile#L199
mokagio
force-pushed
the
remove-deprecated-actions
branch
from
45d20b7 to
fdc15c4
Compare
| def self.run(params) | ||
| require_relative '../../helper/android/android_git_helper' | ||
|
|
||
| Fastlane::Helper::Android::GitHelper.update_metadata(ENV['validate_translations']) |
There was a problem hiding this comment.
The corresponding method in the android_git_helper.rb should also be removed in parallel to removing the action.
Note that this helper method is also marked as @todo so the fact that you missed it makes me wonder if you used case-sensitive search when looking for TODOs/FIXMEs. We should definitively try to be consistent in using @TODO/@FIXME in all uppercase everywhere (I think that's the ruby convention?) but 🤷♂️
PS: As a bonus it would be nice to remove the ./tools/update-translations.sh script (which is the script the update_matadata helper ends up calling) from client repos in parallel to that PR
There was a problem hiding this comment.
Good catch, thanks. Addressed in fae13ff (#337)
As a bonus it would be nice to remove the
./tools/update-translations.shscript
Bonus points to @JavonDavis for doing that a while back:
| # | ||
| # @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` |
There was a problem hiding this comment.
Isn't ios_localize_project still used though (not for long, but until we finish our work on the Localization Tooling Modernization and finally migrate all client apps to the new actions)?
There's another deprecated action still in the codebase: ios_localized_project. It's used in WooCommerce (e.g. here) and Simplenote (which is less pressing because that's on pause).
There was a problem hiding this comment.
Ouch! I'm so glad we have code review 😓
I did run the tests after removing this, but I guess it was a naive of just being happy with that given how old this code is.
…oject`" This reverts commit fa390b6.
mokagio
dismissed
AliSoftware’s stale review
Dismissing because the changes have been implemented.
A little cleanup in preparation for the 3.0.0 release, see #334 (comment).
All the removed actions were unused in our projects (WordPress, WooCommerce, Simplenote, and PocketCasts. I don't have access to Day One and AFAIK Tumblr doesn't yet use the release toolkit), or used only in Simplenote Android which is currently paused.
There's another deprecated action still in the codebase:
ios_localized_project. It's used in WooCommerce (e.g. here) and Simplenote (which is less pressing because that's on pause).I've decided not to remove it to make the 3.0.0 adoption easier. I think Buildkite trigger support and
trunkas default are useful things that we should rollout ASAP, and wouldn't want the friction —hoverer small— of addressing theios_localized_projectdeprecation to block that.