-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
KEP-2853: Branch rename for k/k #3053
Conversation
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 laying out the KEP, I left a few review suggestions 🙏
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/kep.yaml
Outdated
Show resolved
Hide resolved
6633a70
to
76a64b2
Compare
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
/assign @justaugustus @saschagrunert |
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.
Just a few nits, otherwise LGTM :)
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/kep.yaml
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/kep.yaml
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/kep.yaml
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
Signed-off-by: Carlos Panato <ctadeu@gmail.com>
Signed-off-by: Carlos Panato <ctadeu@gmail.com>
47fd2ca
to
859218f
Compare
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md
Outdated
Show resolved
Hide resolved
|
||
- https://github.com/github/renaming | ||
|
||
### Risks and Mitigations |
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.
Have we done any research on how is it going to affect us (speaking of Release Engineering)? For example:
- What changes we might need to do to krel and the release tooling?
- What about the release notes tool?
- What about the version markers (could they be affected by this)?
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.
What changes we might need to do to krel and the release tooling?
What about the release notes tool?
we will need to update some default const we have to use main, or do some fallback if fail, like try master and if fail go to main, or otherwise
What about the version markers (could they be affected by this)?
regarding this maybe @saschagrunert or @justaugustus can give some opinion, I'm not expert in this topic :(
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.
Any tooling would require to be adapted. So there is a risk we miss some of them, which can be mitigated by grepping with multiple eyes :)
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.
will add this as a risk and mitigation plans
nitty-gritty. | ||
--> | ||
- We aim to make the change during the start of v1.25 release (spring 2022). | ||
- Perform a Survey to gather information on downstream consumers and how this might affects their workflow. See [Survey Questions](#survey-questions). |
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 wondering should the survey cover the upstream consumers as well. For example, there might be SIGs and/or projects under kubernetes*
orgs that are affected by this change. IMO, we also want to be sure that they can migrate by the start of the v1.25 release cycle. We should also note that the upstream consumers might be affected by the enhancements and code freeze.
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 can as well.
I already did the first check in the jobs that use k/k and update those to support both branches when using the master name.
but in some jobs we will need to update after the change.
however, if a project uses k/k that is not set in a prow job we will need to discover those and while we are doing the survey can be a good time for that :)
Signed-off-by: Carlos Panato <ctadeu@gmail.com>
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.
@cpanato -- Great work here!
Let's merge and iterate over any remaining feedback, since this is still in provisional
state.
/lgtm
/approve
That said, consider a shorter file name?
keps/sig-release/2853-kubernetes-kubernetes-repository-branch-rename/README.md --> keps/sig-release/2853-k-core-branch-rename/README.md
/hold if you want to address that now
Signed-off-by: Carlos Panato <ctadeu@gmail.com>
did the rename of the directory @justaugustus, PTAL /hold cancel |
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 again for working on this, Carlos!
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cpanato, justaugustus 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 |
authors: | ||
- "@cpanato" | ||
owning-sig: sig-release | ||
participating-sigs: |
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.
In the future I would bring a KEP to participating SIGs before merging it.
This will have impact well beyond SIG Release.
- Perform a Survey to gather information on downstream consumers and how this might affects their workflow. See [Survey Questions](#survey-questions). | ||
- Announce the change in the kubernetes-dev and kubernetes-announcement mailing list and in a blog post. As well as use Twitter and other social media to announce. | ||
- Change all open Pull Requests to `Draft mode` (so that when we rename the branch, tests will not be triggered, and we avoid a massive queue in the CI infrastructure). | ||
- Update all Prow jobs that use `kubernetes/kubernetes` and use the `master` branch to listen to the `main` branch 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.
This is an incomplete answer due to periodics. They don’t listen to a branch, only time, but they must explicitly configure a branch to clone. Thankfully many of them consume a CI build instead, but not remotely all of them.
main
main
#2853@saschagrunert @justaugustus @puerco @xmudrii @Verolop
@kubernetes/release-engineering