-
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
📖 Update CAPI support and guarantees for v1.6 #9119
📖 Update CAPI support and guarantees for v1.6 #9119
Conversation
@g-gaston: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/area documentation |
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 PR, few suggestions inline:
CONTRIBUTING.md
Outdated
@@ -2,35 +2,36 @@ | |||
<!-- START doctoc generated TOC please keep comment here to allow auto update --> | |||
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> | |||
|
|||
- [Contributor License Agreements](#contributor-license-agreements) |
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.
@g-gaston this is correct, can you please update the release-task.md with the notes that we need to update this file 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.
Which change do you mean? The table of contents or the one below with EOL?
EOL is already documented
- For minor releases Set EOL date for previous release (prior art: 📖 book: set v1.1.x EOL date #7146) and update Cluster API support and guarantees in CONTRIBUTING.md (prior art: 📖 docs: update CAPI support and guarantees #8308).
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 think this usually included adding the new release as well (see https://github.com/kubernetes-sigs/cluster-api/pull/8308/files?diff=split&w=1)
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.
@sbueringer I meant the table in this file with release/API version/Supported until notes. Because currently, there is no mention of the need for updating this file in the release-tasks.md
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 confused. I'm literally quoting release-tasks.md in https://github.com/kubernetes-sigs/cluster-api/pull/9119/files#r1284500772 :)
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 just don't literally call out the individual tables I think. Otherwise it's covered
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.
@sbueringer I see now. in release-tasks.md
looks like we are mentioning it twice: https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/release/release-tasks.md#update-supported-versions and in https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/release/release-tasks.md#repeatedly-cut-a-release number 6 in the list. I wonder if we need it in both places, since we are referring (in Prior art
) to update same files in both cases
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.
The idea was:
- At the beginning of a new cycle:
- Add the new version that we are now developing to versions.md
- After we cut the minor release:
- Set the EOL date for the version that is now EOL in contributing.md
- Add the minor release we just created to the same table in contributing.md
Not entirely sure but I think we don't mention it twice, the PRs just have similar names. And in one case we had some cleanup additionally to do in versions.md for some reason (I think because the other task wasn't done as documented before)
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.
done
/cc @chrischdi |
dd445fa
to
62c44ad
Compare
CONTRIBUTING.md
Outdated
@@ -174,7 +175,7 @@ Cluster API maintains the most recent release/releases for all supported API and | |||
|
|||
(*) Previous support policy applies, older minor releases were immediately unsupported when a new major/minor release was available | |||
|
|||
- Exceptions can be filed with maintainers and taken into consideration on a case-by-case basis. | |||
- Exceptions can be filed with maintainers and taken into consideration on a case-by-case basis.giu |
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.
- Exceptions can be filed with maintainers and taken into consideration on a case-by-case basis.giu | |
- Exceptions can be filed with maintainers and taken into consideration on a case-by-case basis. |
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.
@g-gaston still open :)
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.
Apologies Stefan, I committed your suggestion from the UI, I'm not sure how this got lost.
Fixed
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.
No problem
Looks good to me, but leaving lgtm for the folks you are waiting to hear back from. Thank you for putting this together @g-gaston ! |
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
LGTM label has been added. Git tree hash: 10e3643db4feda26adccf059e32df9e7c823aca6
|
d3c931f
to
734ae84
Compare
734ae84
to
f67181c
Compare
f67181c
to
56eb23c
Compare
@g-gaston: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/retest-required |
Last nit: #9119 (comment) Tables are looking good now |
56eb23c
to
4735113
Compare
Thank you!! /lgtm |
LGTM label has been added. Git tree hash: e779d542b25a870a567ab25989a6e30dcd9c51ef
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
Completing the release comms task for this week.
@kubernetes-sigs/cluster-api-release-team
/area documentation