-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
This is ready for review; leaving it as a draft only to prevent it from merging until wider approval. |
cc'ing vendor directory owners - would love to hear your feedback on the policy. cirq-ionq: @dabacon @ColemanCollins @nakardo @gmauricio @Cynocracy Thanks in advance! |
+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? |
@Cynocracy thanks for taking a look!
Not at the moment, but the hope is to get to this soon after Cirq 1.0. Tracking issue: #5745
With this policy it's unfortunately not possible. The main complexity is around the version compatibility matrix if vendor versions were to differ from 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! |
There was a problem hiding this 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!
cirq-google/version_policy.md
Outdated
@@ -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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
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. |
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 |
There was a problem hiding this 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
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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! |
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
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 allowscirq-core
to be stable while giving vendors the agility to move fast if needed.@MichaelBroughton