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.

Sign up for GitHub

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
Merged

Add additional thoughts on rolling out breaking changes #2457

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a21e0a
Add additional thoughts on rolling out breaking changes
kfcampbell May 23, 2023
1234df5
Remove extraneous newline
kfcampbell May 23, 2023
69d8b20
npm run lint:fix
kfcampbell May 23, 2023
675ab42
Add types at top of tree and additional note about breaking changes
kfcampbell May 23, 2023
5180768
Update with @gr2m's thoughts
kfcampbell May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions MAINTAINING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,25 @@ If the pull request looks good but does not follow the commit conventions, use t

![Screenshot of GitHub's Squash and Merge confirm dialog](assets/squash-and-merge-dialog.png)]

## Breaking changes

When merging a breaking change using the PR body as specified above, extra care must be taken. Breaking changes must first be merged into the `beta` branch, where further testing may be conducted and additional breaking changes may be combined before cutting a release. The `beta` branch should be created for this purpose and based on `main`. Once the first commit to the branch is landed, a draft PR should be created from `beta` to `main` with the title vX, where X is the next major version. After changes are combined and tested, mark the PR as ready for review, get it reviewed, and merge the `beta` branch into `main` as documented above. `beta` branches are intended to be short-lived.

Note the repository for the change: if it's dependent on other repos where the same change must be made, merge the leaf nodes first before the nodes higher up the tree. Your merge order should look something like:

1. octokit/types (when type changes are required)
1. endpoint
1. request
1. plugins
1. auth strategies
1. core
1. \*-methods
1. oauth-app
1. webhooks
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.

## Maintenance releases

### 0. Requirements
Expand Down