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

How to handle PRs with breaking API changes between "breaking API" releases #5907

Closed
alamb opened this issue Jun 17, 2024 · 5 comments · Fixed by #5953
Closed

How to handle PRs with breaking API changes between "breaking API" releases #5907

alamb opened this issue Jun 17, 2024 · 5 comments · Fixed by #5953
Labels
question Further information is requested

Comments

@alamb
Copy link
Contributor

alamb commented Jun 17, 2024

Our release schedule is here https://github.com/apache/arrow-rs?tab=readme-ov-file#release-versioning-and-schedule

This implies we need to wait until after our August 2024 release to have any new breaking API changes (aka arrow 53.0.0).

The question is "what do we do between now, and when we release arrow 53.0.0 with PRs that have breaking API changes such as #5845

I can see three options:

Option 1: Hold Them

  • Continue to do all releases off the master branch.
  • Keep any PRs with breaking API changes in draft (or mark it with a label or something) and wait until master opens for the next breaking API release (in this case after the August 2024 release)

Option 2: Merge to a 53.0.0 feature branch

  • Continue to do all releases off the master branch
  • Merge any PRs with breaking API changes to a new integration branch (53.0.0-dev or something)
  • When master opens for 53.0.0 merge all changes from the 53.0.0-dev branch to master

Option 3: Merge to main branch

  • In this model we would do minor releases off separate branches and main would remain open for all PRs
  • Merge all PRs to master (including those with breaking changes)
  • Create branches for minor releases (e.g. a 52.1.0-dev branch) and backport any commits from master that should be included in the release

Analysis of options

I think Option 1 is the simplest but runs the risk of PRs sitting for months, bitrotting, and then a merge-tastrophe when we try to merge many outstanding PRs at once. This would be especially bad if there are PRs with large API changes prone to accumulating conflicts

Option 2 might make the bit-rot problem better, but requires someone to actively keep that branch sync'd with main / resolve any conflicts that arise. This would likely fall on the maintainers to do

Option 3 keeps the workflow simple, but now requires any contributors to create two PRs (one to main and one to the dev branch) to get things out in minor releases

@alamb alamb added the question Further information is requested label Jun 17, 2024
@alamb
Copy link
Contributor Author

alamb commented Jun 17, 2024

I think after the analysis above, I suggest we go with Option 1

@Xuanwo
Copy link
Member

Xuanwo commented Jun 18, 2024

I think after the analysis above, I suggest we go with Option 1

Given the current maintenance status, I recommend choosing Option 1. Please ensure that the contributor is informed about the status in advance.

@alamb
Copy link
Contributor Author

alamb commented Jun 18, 2024

Please ensure that the contributor is informed about the status in advance.

That is a good point -- if we come to consensus on an approach, I will make a PR that updates https://github.com/apache/arrow-rs?tab=readme-ov-file#release-versioning-and-schedule / the contributor guide with the information on breaking API changes

@tustvold
Copy link
Contributor

Option 1 I think makes the right tradeoff of making the process for non-breaking changes as simple as possible

@alamb
Copy link
Contributor Author

alamb commented Jun 24, 2024

Here is a PR documenting Option 1 as the policy: #5953

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants