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

Add MIGRATION.md migration guide #4

Conversation

iangmaia
Copy link
Owner

@iangmaia iangmaia commented Feb 10, 2023

EDIT: This PR has been re-created in the main repo as wordpress-mobile#452

What does it do?

This PR proposes the usage of a migration guide, MIGRATION.md.
The idea is that a guide like this can be specially useful to provide more context for a successful migration between major releases of the release-toolkit, giving more information about deprecations, clean-ups that the clients can do, how to deal with breaking changes and any other adaptations that the hosting projects need to do when migrating to a new version.

A process to update the MIGRATION.md file can be very similar to the one followed by CHANGELOG.md: PRs containing changes that would require attention during a migration can highlight them in the appropriate subsection of the ## Migrating to Trunk section. Once the new major version is released, ## Migrating to Trunk can be changed to ## Migrating to 7.0.0, for example.
I've initially organised the migration topics into two main subsections, Considerations for breaking changes and Clean-ups, which I believe will be common cases; more subsections can be added or a different grouping can be done as we see fit.

This is the discussion with @AliSoftware that originated this PR.

Related PRs

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.

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.

Nice!

I think we need to add a bullet point about the removal of the Deliverfile too (to account for once this PR lands)?

MIGRATION.md Outdated Show resolved Hide resolved
@AliSoftware
Copy link

AliSoftware commented Feb 10, 2023

We also need to document the migration needed for when wordpress-mobile#445 lands (passing xcconfig_file: param + cleanup by removing the need for ENV['PUBLIC_CONFIG_FILE']—or alternatively just replacing ENV['PUBLIC_CONFIG_FILE'] with ENV['FL_IOS_PUBLIC_VERSION_XCCONFIG_FILE'] instead, though we'd still recommended the former solution)


PS: I know that your branch of this PR builds only on top of wordpress-mobile#447 like you mentioned in your PR description… and there was no magic way for you to make a PR that would be on top of all the pending PRs (we have that are still open in parallel) all at once… but I still think it'd be worth to already start the documentation for those future PRs in this initial document, given the special case we have here of you working on a fork and this not making it easy to stack PRs anyway. As long as we document not to merge this PR until all the others have landed because it documents all of them at once, of course.

This will be different once the file has landed in the repo and starts existing, because then future PRs will just add new entries to the existing file like we do with CHANGELOG.md. But for the bootstraping of the file, it's probably simple to document all 3 breaking changes at once in this PR and just land it once all the rest is ready 🙃

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
@iangmaia
Copy link
Owner Author

I think we need to add a bullet point about the removal of the Deliverfile too (to account for once #3 lands)?

We also need to document the migration needed for when wordpress-mobile#445 lands (passing xcconfig_file: param + cleanup by removing the need for ENV['PUBLIC_CONFIG_FILE']—or alternatively just replacing ENV['PUBLIC_CONFIG_FILE'] with ENV['FL_IOS_PUBLIC_VERSION_XCCONFIG_FILE'] instead, though we'd still recommended the former solution)

PS: I know that your branch of this PR builds only on top of wordpress-mobile#447 like you mentioned in your PR description…

Yes, my initial thought was to only refer to the changes done here and perhaps address the additional migration needs once the other PRs get merged.

still think it'd be worth to already start the documentation for those future PRs in this initial document, given the special case we have here of you working on a fork and this not making it easy to stack PRs anyway. As long as we document not to merge this PR until all the others have landed because it documents all of them at once, of course.

This will be different once the file has landed in the repo and starts existing, because then future PRs will just add new entries to the existing file like we do with CHANGELOG.md. But for the bootstraping of the file, it's probably simple to document all 3 breaking changes at once in this PR and just land it once all the rest is ready 🙃

...but these are good arguments 👍 and I agree. Next, I'll add those and will re-create the PR targeting the main repo.

@iangmaia iangmaia deleted the cleanup/remove-legacy-localization-tooling-migration-guide branch February 12, 2023 15:20
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