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

Replace two GitHelper methods with Fastlane actions #531

Merged
merged 6 commits into from Feb 7, 2024

Conversation

spencertransier
Copy link
Contributor

@spencertransier spencertransier commented Dec 29, 2023

What does it do?

This PR deprecates these two methods in the GitHelper module and replaces their uses with built-in Fastlane actions:

  • update_submodules replaced by git_submodule_update
  • ensure_on_branch! replaced by ensure_git_branch

I'm deprecating the methods instead of removing them because we have used some of the GitHelper methods directly instead of through Release Toolkit actions.

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 appropriate existing ### subsection of the existing ## Trunk section.
  • If applicable, add an entry in the MIGRATION.md file to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.

@spencertransier spencertransier requested a review from a team December 29, 2023 22:08
@AliSoftware
Copy link
Contributor

iirc, there was a reason why we had our own ensure_on_branch! instead of ensure_git_branch, as fastlane's implementation was doing something that we didn't want. I think it was something like taking some env vars (like BUILDKITE_BRANCH and such) into account for its check… which didn't work for release-on-CI where the env var still had the old value even after we cut a new branch during the process?) and that's why we re-implemented it ourselves without being affected by the special logic.

I think that now that @oguzkocer implemented a workaround in fastlane in fastlane/fastlane#21597, which has been shipped in fastlane 2.217.0, this is not a problem anymore… as long as your projects have updated to this version of fastlane or later, and also that you do set FL_GIT_BRANCH_DONT_USE_ENV_VARS in your CI environment.

If that's indeed what needs to be done to be ready to migrate, we should include a note about this in the CHANGELOG.md or in the MIGRATION.md file to document it, otherwise this would be breaking projects.

@spencertransier
Copy link
Contributor Author

@AliSoftware I updated the deprecation notice for ensure_on_branch! to add additional context about the Fastlane env var PR. When we're ready to remove it and make it a breaking change, I will add it to MIGRATION.md.

Copy link
Contributor

@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.

Approved with wording tweak suggestions

Co-authored-by: Olivier Halligon <olivier.halligon@automattic.com>
@spencertransier spencertransier merged commit aad3399 into trunk Feb 7, 2024
5 of 7 checks passed
@spencertransier spencertransier deleted the use/built-in-fastlane-actions branch February 7, 2024 15:56
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.

None yet

2 participants