-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
📖 Document breaking changes policy #1637
📖 Document breaking changes policy #1637
Conversation
/cc @akutz |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ncdc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cc @timothysc |
|
||
## Breaking Changes | ||
|
||
Breaking changes are generally allowed in the `master` branch, as this is the branch used to develop the next minor |
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.
We should specify that this applies only if master is open for the next iteration
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 know Kubernetes sends out updates throughout the release cycle informing about the state of the master branch. Maybe we should do something similar (defer to #768?)
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.
That sounds good to me, let's document that as well?
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.
Yes, but as part of 768
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.
Eek, I linked to the wrong thing. Nevermind about 768 😄. We had talked at the f2f about formalizing the release cycle, which is what I was trying to link to, but I don't think we ever created an issue for it (or I can't find it quickly).
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.
Updated PTAL
CONTRIBUTING.md
Outdated
|
||
### Merge Approval | ||
|
||
Cluster API maintainers may add "LGTM" (Looks Good To Me) or an equivalent comment to indicate that a PR is acceptable. Any change requires at least one LGTM. No pull requests can be merged until at least one Cluster API maintainer signs off with an LGTM. | ||
Cluster API maintainers may add "LGTM" (Looks Good To Me) or an equivalent comment to indicate that a PR is acceptable. | ||
Any change requires at least one LGTM. No pull requests can be merged until at least one Cluster API maintainer signs |
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.
This is hard to do in practice, given that any k8s contributor can, in theory add /lgtm to anything
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.
This isn't net new but I can adjust it
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.
Updated PTAL
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.
nice, this looks good to me
59c157a
to
05d3472
Compare
Document breaking changes guidelines in the contributing doc. Also clean up word wrapping. Signed-off-by: Andy Goldstein <[email protected]>
05d3472
to
b3004ab
Compare
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.
/lgtm
What this PR does / why we need it:
Document breaking changes guidelines in the contributing doc. Also clean
up word wrapping.
Note, most of the diffs are just word wrapping the paragraphs, with the addition of the breaking changes section. I also cleaned up the formatting of the list with the PR title emojis.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #1598