streamline CFP approval and tracking process#37
streamline CFP approval and tracking process#37katiestruthers wants to merge 2 commits intocilium:mainfrom
Conversation
Signed-off-by: Katie Struthers <[email protected]>
Signed-off-by: Katie Struthers <[email protected]>
joestringer
left a comment
There was a problem hiding this comment.
Thanks for tackling this. I think that having a set of statuses that contributors can use to label their CFPs is a helpful step in establishing the process. Ultimately I think that these will just have to be tested against reality to prove the usefulness for different stages and to establish precedence about how the community treats CFPs with different statuses, but until then I've provided some minimal feedback.
Given that it's relatively new to the Cilium community to track & review designs using a git repository, I think we may struggle a little bit to clearly define how the overall process should work to balance velocity and scrutiny, so some of my feedback below may not be directly relevant to the aim of this PR (defining status) vs. understanding how this status fits into the wider process. Bear with me as I try to figure this stuff out with you all :)
I dropped a couple of minor wording nits below as well on how to improve some of the wording.
| Cilium Github organization. | ||
| Cilium Github organization. | ||
|
|
||
| All current CFPs are listed in the [Cilium Feature Proposals (CFPs) project]() found in this repository. |
There was a problem hiding this comment.
nit: What information does this communicate that the previous paragraph doesn't communicate? Should it be skipped or combined with the previous paragraph?
There was a problem hiding this comment.
Also, we aren't using GitHub Projects for this currently: https://github.com/cilium/design-cfps/projects?query=is%3Aopen .
There was a problem hiding this comment.
Oh I see, I missed the PR description when I first read the PR 🤦
These statuses will be listed in the Cilium Feature Proposals (CFP) GitHub Project that will be added to this repository. Workflows are set to automatically set Provisional (when a new PR is opened), Implementable (when a PR is merged), and Withdrawn (when a PR is closed), but the other three statuses will have to be manually set. The SIGs will also have to be manually set.
How will we manage these? If this is automated somehow, great. If not, then I think this is creating more busywork that is likely to be neglected by the Cilium project unless you find someone specific who will volunteer to keep the CFP projects up to date. Almost every time we've tried to use projects or milestones etc. in the Cilium project, there's one person who pays attention to them for a few weeks, then they lose interest and the project gets stale and outdated. A big reason I see for this is that the benefits of managing this "virtual paperwork" are not high enough to make it part of our regular working process.
There was a problem hiding this comment.
If this is something that SIG leads see value in and would be happy to help with, then I would encourage setting up something like that. On the other hand, if we don't have buy-in from anyone who would actively engage with this process, then I think it's the process that needs to change to fit the people, not the people changing to fit the process.
There was a problem hiding this comment.
I'll CC in specifically @marseel , @hemanthmalla (on behalf of @cilium/sig-scalability) and @squeed (on behalf of @cilium/sig-policy) in case they have opinions about how to track CFPs within specific SIGs and whether sharing a project to track these things would be helpful for them.
| You are encouraged to share your PR in the [appropriate SIG slack channel](https://docs.cilium.io/en/stable/community/community/#all-sigs) to speed up the approval process. If you want to bring further attention to your design, you may want to | ||
| raise the design during the [weekly community call](https://docs.cilium.io/en/v1.13/community/community/#id1) | ||
| and on the [#development channel in Slack](https://cilium.slack.com/archives/C2B917YHE). |
There was a problem hiding this comment.
The link here helps 👍
nit: To simplify the wording and make it accessible to readers from a variety of backgrounds, I would suggest to use more straightforward imperative wording, such as:
| You are encouraged to share your PR in the [appropriate SIG slack channel](https://docs.cilium.io/en/stable/community/community/#all-sigs) to speed up the approval process. If you want to bring further attention to your design, you may want to | |
| raise the design during the [weekly community call](https://docs.cilium.io/en/v1.13/community/community/#id1) | |
| and on the [#development channel in Slack](https://cilium.slack.com/archives/C2B917YHE). | |
| Promote awareness about your CFP in the community: | |
| - Share it on the [#development channel in Slack](https://cilium.slack.com/archives/C2B917YHE) or [one of the more specific SIG channels](https://docs.cilium.io/en/v1.13/community/community/#id1). | |
| - Raise the design for discussion during the [weekly community call](https://docs.cilium.io/en/v1.13/community/community/#id1). |
sidenote regarding the above feedback, while community members "are encouraged" and they may "want to bring further attention...", this wording is imprecise, and we've adopted a style guide for better communication since the previous version of this doc was written.
|
|
||
| 1. ``Provisional``: All new PRs are given this status. This CFP's design has not yet been approved. | ||
| 2. ``Implementable``: Approved PRs that are merged are given this status. Their CFP design has been approved, and now the work can be started to implement them. | ||
| 3. ``Experimental``: The CFP has been implemented and is now in a 'Beta' state. |
There was a problem hiding this comment.
Is it experimental, or is it beta? These words have different connotations ranging from "Use at your own risk" to "Please run this in production and report your findings".
I recognize the need for a category like this between "Implementable" and "Stable", though I'm not familiar enough with process or how the process impacts human behaviour in order to have a strong opinion on how to portray a proposal in this status.
I can see benefits in leaning it towards wording like "experimental" as a way to incentivize contributors to work on moving the CFP towards "Stable" state, though I could equally see contributors simply ignoring the CFP process after it becomes "Implementable" and completely focusing on stabilizing the functionality to the point where it's production-ready.
I'll CC @youngnick in here in case he has some opinion about how a process like this should look, as I think he has more experience than me on this.
There was a problem hiding this comment.
Related: For the Cilium project, this is likely to differ from other communities like Gateway API, because these designs in Cilium are targeted for typically one implementation in Cilium (one of the repos under this organization), and we have additional expectations about testing, docs, etc for that implementation as we accept changes. On the other hand when the entire project is a standard like Gateway API or Network Policy API in upstream k8s sig-network, then some of those requirements may end up in the design repo.
| 1. ``Provisional``: All new PRs are given this status. This CFP's design has not yet been approved. | ||
| 2. ``Implementable``: Approved PRs that are merged are given this status. Their CFP design has been approved, and now the work can be started to implement them. | ||
| 3. ``Experimental``: The CFP has been implemented and is now in a 'Beta' state. | ||
| 4. ``Stable``: The CFP has been implemented, rigororously tested, and is now in a 'GA' state. |
There was a problem hiding this comment.
I guess related to the previous comment, we have the state of the CFP then the state of the feature. Each of these lines seems to describe the state of the CFP as tracking a specific state of the feature, though these terms like 'beta' or 'GA' are not currently defined anywhere, so it's a little hard to understand what those mean.
Is the rigorous testing intended for the design or the implementation?
| 2. ``Implementable``: Approved PRs that are merged are given this status. Their CFP design has been approved, and now the work can be started to implement them. | ||
| 3. ``Experimental``: The CFP has been implemented and is now in a 'Beta' state. | ||
| 4. ``Stable``: The CFP has been implemented, rigororously tested, and is now in a 'GA' state. | ||
| 5. ``Complete``: All implementation work is complete and has been merged. |
There was a problem hiding this comment.
How is this "Complete" state different from "Stable"?
| 4. ``Stable``: The CFP has been implemented, rigororously tested, and is now in a 'GA' state. | ||
| 5. ``Complete``: All implementation work is complete and has been merged. | ||
|
|
||
| At any time during the process, the CFP can be given the status of ``Withdrawn`` if it is no longer thought to be viable, or if it is not wanted to be pursued for any other reason. No newline at end of file |
There was a problem hiding this comment.
To help inform the thought process, I have a PR (#36) that sort-of fits this category. I labelled it "Dormant" for now, because in this case the design review resulted in "Changes Requested", but I do not currently have the cycles to pursue the change further. The core idea is still worthwhile, but it hasn't proven critical enough to solve just yet. The idea isn't exactly "Withdrawn", but the specific design is practically speaking "Withdrawn". I'm not sure whether I would consider that one to be under "Provisional" or "Withdrawn" given that the primary blocker on moving it forward is just interest from a contributor to resolve the outstanding feedback and push it forward.
|
Katie originally put together a Github project to automate some of this Cilium Feature Proposals (CFPs) (view). However, thinking about this again, since there hasn't been a lot more discussion about what the best process is, it might be a bit of premature optimization. I'll pull out some of the wording improvements and we can refer back to this discussion if/when we want to revisit this in the future. For now, I think we should just focus building out the operations of the SIGs and review this again if we think we need a better process around CFPs. |
Inspired by the Gateway Enhancement Proposal (GEP) docs.
I tried to combine @joestringer's comment in #4 with the three proposed states in that same issue, resulting in 6 possible statuses (I renamed 'State' to 'Status' just to fit the default for GitHub Projects).
ExperimentalandStablecould use some stronger definitions, but I think the other four make sense.These statuses will be listed in the Cilium Feature Proposals (CFP) GitHub Project that will be added to this repository. Workflows are set to automatically set Provisional (when a new PR is opened), Implementable (when a PR is merged), and Withdrawn (when a PR is closed), but the other three statuses will have to be manually set. The SIGs will also have to be manually set.
I have also opened a PR to update the list of SIGs, so those links I added will be more useful soon.