Updating our merge requirements #549
phlogistonjohn
started this conversation in
General
Replies: 1 comment 1 reply
-
I'd also like to mention some of the outcomes of discussion #525 makes this change land a bit more softly because not requiring APIs to be locked in stone [1] if they're gated by a build tags means we can still make some changes to them after a release. [1] - not that they ever were, but I/we often push back on minor changes to an API after a release because there's no good way to know that we're not breaking code by changing an api. Now we've got an excuse. |
Beta Was this translation helpful? Give feedback.
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
In order to improve how we get code merged in go-ceph, and to make the process more pleasant and regular I'm making the following proposals to change how we/when we can merge code:
For code that does not impact our public API - we only require 1 approving review and no requested changes. We've already started doing this somewhat unofficially.
For code that does impact our public API - we require 1 approving review and either
a 2nd approving review
or10 days to pass
[1], and no requested changes.If the submitter or a maintainer/reviewer wishes, the PR can also be labeled "extended-review" this means the content is extra large, subtle, or simply needs extra scrutiny for some reason. As before having this label should disable any automerge tools.
I'd like to implement as much of the policy using automation as possible but it is not strictly required. If there isn't a lot of discussion or disagreement on this I plan on starting to officially implement this policy in a 2-3 weeks after the next release (expected on 2021-08-10).
[1] - Why 10 days? It's longer than a week, and folks often do go-ceph work for $dayjob, and taking a week off is common. But its not much longer than a week. :-)
Beta Was this translation helpful? Give feedback.
All reactions