-
Notifications
You must be signed in to change notification settings - Fork 5.2k
KEP metadata #1135
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 metadata #1135
Conversation
Thanks @jbeda. I'm not sure whether folding the instructions into the template makes it easier or harder to use. Usually I find it annoying to have to delete a bunch of text every time I want to use a template. |
I'm guessing that we should optimize for the case where users don't do this all the time. Keeping the template instructions in line with the template will be a bit of a pain. However, willing to go either way here. |
The template doc doesn't ever say what KEP stands for. |
@mikedanese See https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/kubernetes-enhancement-proposal-process.md. Lots of ongoing discussion in SIG-architecture around this. |
@jbeda Are you planning to restructure this, or would you like a detailed review in its current form? |
I'm going to update this today. Sorry for the delay! |
OK -- reorganized with clean commit history. Happy to pull off the last two commits to a separate PR if that helps clarify things. @bgrant0607 |
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, @jbeda. That helped. Just a few minor comments. Clarifying the merge criteria is most important, IMO, but I think everything could be addressed in subsequent PRs.
* Make a copy in the appropriate directory. Name it `draft-YYYYMMDD-my-title.md`. | ||
* Create a PR in the | ||
[`kubernetes/community`](https://github.com/kubernetes/community) repo. | ||
* Check in early. View anything marked as a draft as a working document. By |
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.
It would be useful to indicate minimum requirements for merge.
I assume the proposal still needs to be reviewed prior to merge? There's no good way to use github to review content that has already been merged AFAIK. One needs to open a PR that proposes specific changes.
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.
Tweaked it a bit. Take a look when I push.
As for comments -- I'm thinking we view this as "PRs accepted". If something doesn't work submit a PR to fix it and discuss in that.
KEP filename. See the template for instructions and details. | ||
* **status** Required | ||
* The current state of the KEP. | ||
* Must be one of `Draft`, `Deferred`, `Accepted`, `Rejected`, `Withdrawn`, |
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 fine with iterating on these stages in subsequent PRs, but I still think the "Accepted" stage is unnecessarily inconsistent with existing project terminology, which would be "Approved".
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 picky. Changed to Approved.
* A list of authors for the KEP. We require a name (which can be a psuedonym) | ||
along with a github ID. Other ways to contact the author is strongly | ||
encouraged. This is a list of maps. Subkeys of each item: `name`, | ||
`github`, `email`, `slack`. |
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 we should defer email to later. It opens up a can of worms.
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 marking it as optional. Do you think that we should remove it altogether?
@@ -1,46 +1,185 @@ | |||
[//]: # ( thank you for creating a KEP! ) | |||
[//]: # ( read the suggested section content: https://github.com/calebamiles/community/blob/propose-kep-template/contributors/design-proposals/architecture/kep-template-instructions.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 think this link to instructions would be worth keeping
I think everything is addressed. Let me know if you have more changes. Next step after this is to adjust the state machine to something we are all happy with. |
Good enough for now. Thanks. Email address: We've run up against github's privacy policy in the past. /lgtm |
Automatic merge from submit-queue. |
Automatic merge from submit-queue. KEP metadata Markdownify KEP metadata proposal that was discussed in https://docs.google.com/document/d/1ynmBMuDuT7yGzRscObB1KtgJj8ljYq0I5q4oshrJUCs/edit?ts=59c1b868#. Also moved files around so that we are following some of the suggestions here. Merged the KEP template and the template instructions to reduce the number of files we need to manage.
Markdownify KEP metadata proposal that was discussed in https://docs.google.com/document/d/1ynmBMuDuT7yGzRscObB1KtgJj8ljYq0I5q4oshrJUCs/edit?ts=59c1b868#.
Also moved files around so that we are following some of the suggestions here. Merged the KEP template and the template instructions to reduce the number of files we need to manage.