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

Post 1.0 version policy and release process #5747

Merged
merged 3 commits into from
Jul 18, 2022

Conversation

verult
Copy link
Collaborator

@verult verult commented Jul 13, 2022

Initial proposal for version policy and release process post-1.0.

The policy requires that cirq-core strictly follows Semantic Versioning and disallow backward-incompatible changes across minor version bumps, but leaves room for each vendor directory to make a separate choice on whether or not to maintain backward compatibility. This allows cirq-core to be stable while giving vendors the agility to move fast if needed.

@MichaelBroughton

@verult verult requested a review from MichaelBroughton July 13, 2022 02:31
@CirqBot CirqBot added the size: M 50< lines changed <250 label Jul 13, 2022
@verult
Copy link
Collaborator Author

verult commented Jul 13, 2022

This is ready for review; leaving it as a draft only to prevent it from merging until wider approval.

@verult
Copy link
Collaborator Author

verult commented Jul 14, 2022

cc'ing vendor directory owners - would love to hear your feedback on the policy.

cirq-ionq: @dabacon @ColemanCollins @nakardo @gmauricio @Cynocracy
cirq-aqt: @ma5x @pschindler @alfrisch
cirq-pasqal: @HGSilveri
cirq-rigetti: @erichulburd @kalzoo @dbanty

Thanks in advance!

@Cynocracy
Copy link
Contributor

Cynocracy commented Jul 14, 2022

+1 from me!

Will there be any automated checking of semantic version backcompat at the API / serializable object level?

One point that I don't fully understand, if cirq-core controls the release schedule and is bound by semver, what does the release process look like when a vendor wants to bump the major/minor independently (say, earlier than the schedule)? Is that possible?

@verult
Copy link
Collaborator Author

verult commented Jul 14, 2022

@Cynocracy thanks for taking a look!

Will there be any automated checking of semantic version backcompat at the API / serializable object level?

Not at the moment, but the hope is to get to this soon after Cirq 1.0. Tracking issue: #5745

One point that I don't fully understand, if cirq-core controls the release schedule and is bound by semver, what does the release process look like when a vendor wants to bump the major/minor independently (say, earlier than the schedule)? Is that possible?

With this policy it's unfortunately not possible. The main complexity is around the version compatibility matrix if vendor versions were to differ from cirq-core. The policy allows this to be changed if we do figure out the compatibility story in the future: "Packages may have different version numbers in the future, at which point this policy will be updated." Let me know if the policy wording should be clarified in any way.

That being said, there may be rare occasions when an urgent patch is necessary for any of the packages, and releasing from the main development branch will be unstable. I'm drafting a one-off process we could potentially follow, but in the mean time open to suggestions!

Copy link
Contributor

@Cynocracy Cynocracy left a comment

Choose a reason for hiding this comment

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

One concern I would have as a user/developer is formally defining "backward compatibility". All previously working invocations must work? No deleted methods? It's easy to get side tracked in discussions about what we mean by compatibility without formalizing it.

LGTM, I appreciate the flexibility this grants vendors!

@@ -0,0 +1,3 @@
# cirq-google Version Policy

The `cirq-google` directory will mostly abide by Semantic Versioning 2.0.0. In rare cases, we may introduce backward-incompatible changes to the public API in a release with a minor version increment. Such changes will be noted in `CHANGELOG.md` within the directory before the release.
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of "will mostly", "should" might be clearer (following https://datatracker.ietf.org/doc/html/rfc2119 )

backward-incompatible changes for a minor version increment.

A major version increment process has not been established. Until then,
backward-incompatible changes are not allowed for `cirq-core` and vendor
Copy link
Contributor

Choose a reason for hiding this comment

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

I might be nit picking but "should not be allowed" feels more accurate absent an enforcement mechanism

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure about this one - IMO a released backward-breaking change which is discovered later should be considered a bug and be fixed in a subsequent release. Although an enforcement mechanism such as CI is a lot more accurate than enforcement through maintainer reviews, any system could have bugs that let some breaking changes through, and I think the stronger "are not allowed" should be used when either of the two mechanisms are in place.

WDYT?

@verult
Copy link
Collaborator Author

verult commented Jul 15, 2022

One concern I would have as a user/developer is formally defining "backward compatibility". All previously working invocations must work? No deleted methods? It's easy to get side tracked in discussions about what we mean by compatibility without formalizing it.

Yeah totally agree this is very important and these discussions could easily come up with future code changes. I've also tracked this question in #5745, and this needs to be addressed soon so as to not block PR progress after Cirq 1.0.

@verult
Copy link
Collaborator Author

verult commented Jul 15, 2022

Since urgent patches will happen rarely, the process won't be included in this initial version of the policy and. The effort to document a process is tracked in #5745

Copy link
Collaborator

@HGSilveri HGSilveri left a comment

Choose a reason for hiding this comment

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

+1 from me too, I only have a couple remarks/questions

Comment on lines +33 to +42
4. Cirq vendor directories (`cirq-aqt`, `cirq-google`, `cirq-ionq`, etc.) follow
Semantic Versioning except the MINOR version increment policy: each vendor
directory has a separate policy on whether MINOR version increments provide
backward-compatibility guarantees, as described in `version_policy.md` in the
respective directory.
1. If `version_policy.md` does not exist in a particular vendor directory,
MINOR version increments may contain backward-incompatible functionality
changes to its public API.
2. For each vendor directory, version policies may be modified to strictly
follow Semantic Versioning in the future.
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, if I understand correctly, the version of the vendor packages will still track the version of cirq-core, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your review! Yes that's right.

Comment on lines +69 to +71
Releases are made on an as-needed basis determined by Cirq maintainers. All Cirq
packages (including vendor packages such as `cirq-aqt`) are released at the same
time.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok with this approach, though it is unclear how things would unfold should a vendor package require an urgent patch because it seems that cirq-core dictates the release schedule. I agree it should be a rare event, but I would still like to see it documented in the "one-off process" you mentioned above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree that this process needs to be documented. I have a proposal (in short, making a minor version release branch when a patch is necessary) but it still needs to be discussed with a wider audience. Because this is rare, the plan is to defer the discussion to unblock releasing the version policy.

@verult verult marked this pull request as ready for review July 15, 2022 18:44
@verult verult requested review from wcourtney, a team, vtomole and cduck as code owners July 15, 2022 18:44
@verult
Copy link
Collaborator Author

verult commented Jul 15, 2022

Hi everyone, we are aiming to include the policy in the Cirq 1.0 release - if you'd like to chime in on the policy, it would be greatly appreciated if you could leave a comment by EOD today Pacific time. Thanks to everyone who has reviewed so far!

@MichaelBroughton MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 18, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 18, 2022
@CirqBot CirqBot merged commit 9f5fb8f into quantumlib:master Jul 18, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 18, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
Initial proposal for version policy and release process post-1.0.

The policy requires that `cirq-core` strictly follows Semantic Versioning and disallow backward-incompatible changes across minor version bumps, but leaves room for each vendor directory to make a separate choice on whether or not to maintain backward compatibility. This allows `cirq-core` to be stable while giving vendors the agility to move fast if needed.

@MichaelBroughton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants