Skip to content
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

Should we merge PRs when they are not "Implementable"? #48

Open
joestringer opened this issue Aug 9, 2024 · 8 comments
Open

Should we merge PRs when they are not "Implementable"? #48

joestringer opened this issue Aug 9, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@joestringer
Copy link
Member

We've recently clarified status for CFPs (See #42).

While attempting to apply this standard to existing proposals, I noticed that there may be interest in merging CFPs when they are not considered "Implementable", but the current process largely assumes that this is the minimum bar to merge.

More specifically, there are a couple of cases I'm considering:

Dormant Status

These may require subsequent discussion to identify and resolve key questions prior to marking the CFP as "Implementable". I've already merged #7 under this category, and I think #36 also applies under this category. The status text does not currently really describe one way or another whether such PRs should be merged, but it seems like a reasonable way to "close the loop" on an open PR. The alternative I see would be to just close dormant CFP proposals, and if we wish to adopt them in future then someone would have to resurrect the PR afresh.

Draft Status

The current text prescribes that one of the properties of a draft is that it's not merged. It also describes "Implementable" PRs as being merged. This means that a specific idea needs to be significantly matured before it is merged into the repository. I noticed in #32 that contributors have been managing review discussions in their own public repositories in order to more easily iterate on the discussion. As a pattern, this can work, but it also draws the discussion away from the central official repositories, making them generally less visible to the overall community.

Furthermore when I reviewed the above PR, I had dozens of resulting comments. With so much activity, the GitHub UI tends to begin to inhibit healthy conversation by being slow, making it hard to find comments, and so on. This makes me wonder whether we should redefine the "Draft" status such that we could merge proposals in draft status as long is the core idea has high level agreemeent. We could set the culture to expect "TODOs" to be documented in a draft CFP before merge. Then, before the proposal can graduate to to "Implementable" we would need to ensure that key questions have reasonable solutions and have no major flaws and no significant lack of consensus.

Overall with this proposal, if we had community members working closely together, I could imagine staging the development of a CFP where an initial version just describes the summary and goals to establish high level consensus on the core idea, then subsequent PRs could develop the idea, each of which could undergo some degree of review to merge, then as key ideas/questions/impacts are introduced, even those could be separately committed, and only once the CFP gets to a point where there is rough consensus and committers believe it has been sufficiently explored, then a subsequent PR could move it to "Implementable". While I don't know for sure whether contributors are looking for this degree of incremental progress on CFPs given that CFPs are often written proposed by a single individual, I would be open to exploring mechanisms like this for more incremental progress.

@joestringer joestringer added the enhancement New feature or request label Aug 9, 2024
@joestringer
Copy link
Member Author

@xmulligan @youngnick @bowei @robscott may be interested to provide input on this. Probably the change to wording / process is shorter than the description of the idea above 😅 .

@xmulligan
Copy link
Member

One idea could be to add something like an Idea status.

Can be used as an initial version that describes the summary and goals to establish high level consensus on the core idea. Subsequent PRs should develop the idea to address key ideas/questions/impacts as they are introduced. When there is rough consensus and committers believe the idea has been sufficiently explored, a subsequent PR should move it to Implementable.

@xmulligan
Copy link
Member

I would maybe ask how long something would sit in this "idea" stage before it would go dormant, but maybe that isn't a big concern if it is already documented.

@robscott
Copy link

@xmulligan @youngnick @bowei @robscott may be interested to provide input on this. Probably the change to wording / process is shorter than the description of the idea above 😅 .

Thanks for looping me in to the conversation! While I think it can be valuable to merge proposals before they're "implementable", I think it's important to ensure the following first:

  1. Project maintainers are broadly supportive of the overall idea and consider it in scope for the project if unresolved issues can be worked out. (Merging without this leads to false hope and wasted efforts)
  2. Unresolved items have been clearly identified and are marked with some kind of standard keyword - Kubernetes uses UNRESOLVED for this. Ideally these blocks should provide a summary of any existing discussion along with links to relevant comment threads or docs.

The second point is particularly useful for follow ups. It helps separate the problem into clear follow up PRs that can each tackle a subset of unresolved problems, and it helps reviewers because otherwise it's very difficult to keep track of which parts of a proposal aren't quite settled yet.

@bowei
Copy link
Member

bowei commented Aug 15, 2024

+1 to Rob's comments.

@youngnick
Copy link
Contributor

We have the Idea state in Gateway API, but there we call it Provisional. Same idea though - with all of @robscott's comments applying.

@joestringer
Copy link
Member Author

Provisional state sounds good 👍 I agree that the differentiator between draft to provisional would be that there is a general sense of support for the idea that it is within scope and should be done. The "Unresolved" process also sounds like it would help to resolve my concerns about how we track open discussion items in a provisionally-accepted CFP.

@joestringer
Copy link
Member Author

I'm idly wondering whether "dormant" and this sense of "provisional" state have more overlap than I thought. But I'm also fine to just leave dormant there for now and say that the next step after dormant is provisional - ie a dormant CFP has fewer guarantees about alignment than provisional. At least with provisional I'd expect that there is active discussion about the idea and someone planning to address the feedback. The current cases for "dormant" seem to be more like a good idea that needs someone to own it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants