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

Remove Deliverfile related functionality #3

Conversation

iangmaia
Copy link
Owner

@iangmaia iangmaia commented Jan 29, 2023

EDIT: This PR was re-created in the main repo as wordpress-mobile#450

What does it do?

On this discussion I had with @AliSoftware he mentioned that using the Deliverfile to configure the app version became obsolete, so this PR removes all references to it.

It implements:

  • Removal of the include_deliverfile parameter in Ios::GitHelper.commit_version_bump() and all call sites.
  • Removal of the skip_deliver ConfigItem in the actions ios_bump_version_hotfix and ios_bump_version_release.rb.
  • Removal of the method Ios::VersionHelper::update_fastlane_deliver().

Related PRs

Next steps

The idea is that the current Ios::GitHelper.commit_version_bump() will become obsolete, given now all it does is committing the hard-coded /config/ folder. So based on this work, we can also remove Ios::GitHelper.commit_version_bump() and commit the .xcconfig directly from where it's being changed.

Checklist before requesting a review

  • Run bundle exec rubocop to test for code style violations and recommendations
  • Add Unit Tests (aka specs/*_spec.rb) if applicable
  • Run bundle exec rspec to run the whole test suite and ensure all your tests pass
  • Make sure you added an entry in the CHANGELOG.md file to describe your changes under the approprioate existing ### subsection of the existing ## Trunk section.

@iangmaia iangmaia changed the title Removes Deliverfile related functionality Remove Deliverfile related functionality Jan 29, 2023
Copy link

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

Left a small note about the title of a spec, otherwise looks good!

Note: reminder to reviewers that we might want to wait for us to ensure all the client apps have been updated to stop relying on the Deliverfile to provide the app_version anymore first, before we can land this change.

Better have the client apps ready before we ship the major version of the toolkit, rather than those client apps being blocked from updating the release-toolkit (e.g. if we need a new thing that will land in the release-toolkit after that change, but the client were still relying on the app_version being defined in Deliverfile, but we might not have time/priority to fix that aspect in the client then…)

spec/ios_bump_version_release_spec.rb Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Olivier Halligon <[email protected]>
@iangmaia iangmaia deleted the branch cleanup/remove-legacy-localization-tooling February 10, 2023 18:46
@iangmaia iangmaia closed this Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants