Re: commitfest.postgresql.org is no longer fit for purpose - Mailing list pgsql-hackers
On Fri, May 17, 2024 at 9:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Friday, May 17, 2024, Joe Conway <mail@joeconway.com> wrote:
>> A solution to both of these issues (yours and mine) would be to allow
>> things to be added *during* the CF month. What is the point of having a
>> "freeze" before every CF anyway? Especially if they start out clean. If
>> something is ready for review on day 8 of the CF, why not let it be added
>> for review?
> In conjunction with WIP removing this limitation on the bimonthlies makes
> sense to me.
2. As I mentioned a bit ago, the original idea was that we didn't exit
a CF until it was empty of un-handled patches, so obviously allowing
new patches to come in would mean we'd never get to empty. That idea
didn't work and we don't think that way anymore.
So yeah, I'm okay with abandoning the must-submit-to-a-future-CF
restriction.
Concretely I'm thinking of modifying our patch flow state diagram to this:
stateDiagram-v2
state "Work In Process (Not Timeboxed)" as WIP {
[*] --> CollaboratorsNeeded : Functional but\nFeedback Needed
[*] --> NeedsReview : Simple Enough for\nSign-Off and Send
[*] --> ReworkInProgress : via Returned With Feedback
CollaboratorsNeeded --> NeedsReview : Collaboration Done\nReady for Sign-Off
CollaboratorsNeeded --> WaitingOnAuthor : Feedback Given\nBack with Authors
ReworkInProgress --> ReworkCompleted : Rework Ready\nfor Inspection
ReworkCompleted --> ReworkInProgress : More Changes Needed
ReworkCompleted --> ReadyForCommitter : Requested Rework Confirmed\nTry Again to Commit
NeedsReview --> ReadyForCommitter : Reviewer and Author\nDeem Submission Worthy
NeedsReview --> WaitingOnAuthor : Changes Needed
WaitingOnAuthor --> NeedsReview : Changes Made
WaitingOnAuthor --> CollaboratorsNeeded : Need Help Making Changes
WaitingOnAuthor --> Withdrawn : Not Going to Make Changes
Withdrawn --> [*]
}
state "Bi-Monthly Timeboxing" as BIM {
[*] --> CommitPending : Simple Committer Patch
CommitPending --> Committed : Done!
CommitPending --> ChangesNeeded : Minor Feedback Given
CommitPending --> ReturnedWithFeedback : Really should have been WIP first
ReadyForCommitter --> ChangesNeeded : Able to be fixed\nwithin the current cycle
ReadyForCommitter --> Committed : Done!
ReadyForCommitter --> ReturnedWithFeedback : Not doable in the current cycle\nSuitable for rejections as well
ChangesNeeded --> ReadyForCommitter
Committed --> [*]
ReturnedWithFeedback --> [*]
}
state "Work In Process (Not Timeboxed)" as WIP {
[*] --> CollaboratorsNeeded : Functional but\nFeedback Needed
[*] --> NeedsReview : Simple Enough for\nSign-Off and Send
[*] --> ReworkInProgress : via Returned With Feedback
CollaboratorsNeeded --> NeedsReview : Collaboration Done\nReady for Sign-Off
CollaboratorsNeeded --> WaitingOnAuthor : Feedback Given\nBack with Authors
ReworkInProgress --> ReworkCompleted : Rework Ready\nfor Inspection
ReworkCompleted --> ReworkInProgress : More Changes Needed
ReworkCompleted --> ReadyForCommitter : Requested Rework Confirmed\nTry Again to Commit
NeedsReview --> ReadyForCommitter : Reviewer and Author\nDeem Submission Worthy
NeedsReview --> WaitingOnAuthor : Changes Needed
WaitingOnAuthor --> NeedsReview : Changes Made
WaitingOnAuthor --> CollaboratorsNeeded : Need Help Making Changes
WaitingOnAuthor --> Withdrawn : Not Going to Make Changes
Withdrawn --> [*]
}
state "Bi-Monthly Timeboxing" as BIM {
[*] --> CommitPending : Simple Committer Patch
CommitPending --> Committed : Done!
CommitPending --> ChangesNeeded : Minor Feedback Given
CommitPending --> ReturnedWithFeedback : Really should have been WIP first
ReadyForCommitter --> ChangesNeeded : Able to be fixed\nwithin the current cycle
ReadyForCommitter --> Committed : Done!
ReadyForCommitter --> ReturnedWithFeedback : Not doable in the current cycle\nSuitable for rejections as well
ChangesNeeded --> ReadyForCommitter
Committed --> [*]
ReturnedWithFeedback --> [*]
}
This allows for usage of WIP as a collaboration area with the side benefit of CI.
Patches that have gotten commit cycle feedback don't get lumped back into Needs Review
There is a short-term parking spot for committer-reviewed patches that just cannot be pushed at the moment. That should be low volume enough to cover both quick-fixes and freeze-driven waits.
Collaboration Needed should include a description of what kind of feedback or help is sought. Even if that is just "first timer seeking guidance".
The above details 5 new categories:
Collaborators Needed - Specialized Needs Review for truly WIP work
Rework Completed - Specialized Needs Review to ensure that patches that got into the bi-monthly once get priority for getting committed
Commit Pending - Specialized Needs Review for easily fast-tracked patches; and a parking lot for frozen out patches
Rework in Progress - Parking lot for patches already through the bi-monthly and currently being reworked most likely for the next bi-monthly
Changes Needed - Specialized Waiting on Author but the expected time period or effort to perform the changes is low; or the patch is just a high priority one that deserves to remain in the bi-monthly in order to keep attention on it. When the author is done the committer is waiting for the revisions and so it goes right back into ReadyForCommitter.
I removed Rejected here but it could be kept. It seems reasonably rolled into Returned with Feedback and we shouldn't be rejecting without feedback anyway. Not enough rejection volume to warrant its own category.
David J.
Attachment
pgsql-hackers by date: