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 additional thoughts on rolling out breaking changes #2457

Merged
merged 5 commits into from
May 24, 2023

Conversation

kfcampbell
Copy link
Member

Today we talked about how to roll out breaking changes safely in the octokit.js ecosystem. This PR captures that discussion in a more durable place.

@kfcampbell kfcampbell requested review from gr2m and nickfloyd May 23, 2023 22:41
@kfcampbell kfcampbell added the Type: Documentation Improvements or additions to documentation label May 23, 2023
@wolfy1339
Copy link
Member

It may be good to note that for changes requiring type changes, @octokit/types needs to be updated

1. app
1. octokit/octokit.js
1. octokit/rest.js

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we (or should we) have a procedure for how often we diff the beta branch with main and create new releases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the two shouldn't diverge, beta branches should be short lived. Or is your question about something else?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think there is a procedure, however one should create a draft PR for the next major version merging beta into main, ex: octokit/request.js#586 and then mark it ready for review when it is ready

Copy link
Member Author

Choose a reason for hiding this comment

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

In that case, should I amend the procedure to contain something like:

"After the breaking changes are combined and tested in the beta branch, merge the beta branch into main using a PR with "BREAKING CHANGE" in the body as documented above. beta branches are intended to be short-lived."

Copy link
Member Author

Choose a reason for hiding this comment

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

I'ved added similar language to the PR; please let me know what you think!

Copy link
Contributor

Choose a reason for hiding this comment

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

Good pony by @wolfy1339, we should create beta pull requests as drafts until all planned breaking changes have been merged into it

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add that

  1. beta branches should be deleted first and re-created based on main
  2. Once the first pull request is merged into beta, create a draft pull request from beta to main with the title vX where X is the next breaking version.
    3 Once all breaking changes are merged into beta, test and set the pull request to ready for review

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good! I've now done so.

1. app
1. octokit/octokit.js
1. octokit/rest.js

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the two shouldn't diverge, beta branches should be short lived. Or is your question about something else?

@kfcampbell
Copy link
Member Author

Good idea @wolfy1339! Would that go at the top of the chain since it's a dependency of many other modules?

@wolfy1339
Copy link
Member

That sounds right

@kfcampbell
Copy link
Member Author

I've updated the list accordingly!

@kfcampbell kfcampbell merged commit 2d632d2 into main May 24, 2023
@kfcampbell kfcampbell deleted the breaking-changes-maintenance branch May 24, 2023 17:03
@github-actions
Copy link

🎉 This PR is included in version 3.0.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 2.0.19 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @beta released Type: Documentation Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants