Discussion: Ideas on how to allow better patch organization (parking lot/drafts/requesting feedback/more) #17
Comments
|
I thought the discussion was to have the parking lot be a "fake commitfest", not be a status in an existing one? Or did I misundestand that? |
|
You're could very well be right. This was not my idea, so I might have misinterpreted what exactly was meant. That sounds like quite a bit of work to implement though:
And based on the discussion at FOSDEM I'm not entirely convinced that everyone was even in favor of this idea. Adding a new status seems a lot lower effort to implement, and we could group them in a separate table on the commitfest page. Just like we do for the closed patches already. That would allow for some feedback gathering on the feature. If it then turns out people really want a separate "fake commitfest"/"parking lot" page then we can implement that then. |
Yes, that's what was proposed. (It was not the only thing that was proposed, but when I mentioned the "parking lot" at FOSDEM I was explicitly referring to the idea of a separate CF slot.) |
|
I would suggest naming this "Work in Progress" (WIP) though, and then also adding a new status: "Seeking Input" The existing "Waiting for Author" then means the author doesn't want people reviewing the WIP. The new one is basically where the author isn't ready to ask for a formal review to get the patch into "Ready to Commit" but still needs feedback in order to polish the patch. David J. |
|
I don't thinking having a "Graveyard" permanent CF is all that desirable so I suppose the process flow for withdrawn/rejected etc... would be to move the patch to the Future CF and immediately mark its status appropriately. David J. |
|
If we're bikeshedding about naming, then I'd go for "Draft" it's short and to the point, and it alligns with GitHub's naming which many people are familliar with. |
|
As shortly discussed on Discord, it might be nice to allow patches in this state to be created without any email link so people could first create the cf entry before sending the initial email of the thread. |
|
I like “Draft” |
I think such a patch belongs in a regular CF, then, not the Parking Lot. ("Needs early design feedback" is the sort of thing I'd like to see in a label rather than a patch state, personally.) |
|
I envision that the only patches that ever get into a periodic time-boxed CF are those that are marked Ready to Commit. They may go back to Waiting on Author or Needs Reviews but would never start out their life in the time-box in those states. Nearly every patch begins its life in Draft, waiting to either be formally Reviewed or marked as just wanting feedback. I haven't looked at the Label proposal but maybe... I figured we already have statuses implemented so less friction to just make a new one. A label probably should be a persistent attribute of a patch while retaining statuses for things that change; but I haven't worked through the details. |
|
@polobo Maybe -- I'm not a fan of that vision, personally -- but it's very far away from the "parking lot" request that's been made several times over the past year-plus. If it's decided that the parking lot isn't actually what the community actually wants, that's fine by me -- but fundamentally changing the CF workflow is a much bigger lift that I'm not signing up for with my PR. 😁 |
|
Well, your link to "proposed" is my CF workflow email. If you have something else in mind I'm missing where its been communicated to the public as a proposal. I'm not understanding what analogy is being conveyed by "Parking Lot". You've basically described the "Draft" proposed workflow in your PR so I figured this was the workflow that motivated working on this feature. It's putting the cart before the horse to implement new CF features if we don't have them integrated in a workflow shared with the community. For really low hanging fruit just adding a status of "Requesting Feedback" in between "Waiting on Author" and "Needs Review" would probably solve many of the complaints. If looking for stuff to make committable, filter on Needs Review. If looking to help others move early draft work stuff along, filter on Requesting Feedback. I don't see a meaningful distinction between "Waiting on Author" and "Parking Lot" if we are talking about statuses. The permanent CF reduces the shuffle burden for authors but that is a separate goal. It could make the main CF more manageable, and implicitly gets you an importance distinction - either because the patch itself is important, say a bug fix, or because the patch has been reviewed, deemed ready for committer, and then moved back into review. This last item being more fuzzy and the former could be handled in other ways. In short, if it isn't easy maybe a separate CF should be a future plan after seeing if statues can be used within the new workflow. David J. |
|
Reviewing comments some more: parking lot just seems like a special "waiting on author" condition for patches that haven't made it to the first review request. The name discourages external feedback. It doesn't really add anything new beyond a slightly smaller CF. That is useful but IMO too narrow a role in the workflow. And assuming all statues are available in all CFs the more versatile workflow is already possible anyway. The main decision point, IMO, is whether to encourage moving patches to the open CF when they are ready for review or once they are ready-to-commit. Its a policy, not a technical, concern - the system should naturally handle either decision; and exceptions for individual patches. Also, statues and labels probably should be orthogonal in application but "requesting feedback" doesn't really fit in that way. It implies "waiting on author". |
I intended to link to the beginning of the conversation where it was discussed, but I see that I forgot to link the "flat" version of the list, so that's confusing. Sorry. https://www.postgresql.org/message-id/2186208.1715895987%40sss.pgh.pa.us:
I'm interested in implementing the first half of Tom's sentence there, because I agree there's enough consensus (not unanimity, obviously) to at least try it. |
|
Pulling in @JelteF Maybe it would help if I simply filed a new issue to correspond to my PR? I don't want to squash brainstorming/discussion on a draft state for patches, if that's really what this issue is supposed to be about -- but I emphatically do not want to work on that. 😁 I'm not interested in that UX at all. What I want, selfishly, is a parking lot, and I'm willing to rage-code it into existence to see if people like it. |
|
Fair. I think my perspective now is that this fixed CF is mechanically challenging and useful. We can quibble over naming later but doing this work and having this in some form is indeed a step forward. Thank you for the work. |
Sounds great to me. That's how stuff gets done. I definitely think we need something to track/mark "patches that authors want to publish and keep track of, but not receive tons of reviews on". I'm not sure if this "Parking lot" idea is the best, but I talked to multiple people that seemed into the idea. So I'd say let's try it out and refine it from there. |
JelteF
changed the title
|
I'm not the type of person that thinks every PR needs a matching issue. I'd say let's keep this open for now as a discussion topic (I changed the title) and discuss the actual parking lot implementation & details on your PR. |
|
I wouldn’t worry about the conversation here, mostly it’s me repeating my last email on the topic until people give in and agree with me. |
That works too. |
|
Sounds good; I will avoid adding any "closes" tags to the PR. |
Some people just want to run CI on some work in progress they are doing. They don't expect others to review it, but "waiting for author" is also a bit vague.
The text was updated successfully, but these errors were encountered: