Thread: Commitfest documentation

Commitfest documentation

From
Jehan-Guillaume de Rorthais
Date:
Hi,

In the commitfest application, I was wondering today what was the exact meaning
and difference between open/closed status (is it only for the current
commitfest?) and between «waiting for author» and «Returned with feedback».

I couldn't find a clear definition searching the wiki, the mailing list (too
much unrelated results) or in the app itself.

Maybe the commitfest home page/menu could link to some documentation hosted in
the wiki? Eg. in https://wiki.postgresql.org/wiki/Reviewing_a_Patch

Thoughts? Pointers I missed?

Regards,



Re: Commitfest documentation

From
Aleksander Alekseev
Date:
Hi Jehan-Guillaume,

> In the commitfest application, I was wondering today what was the exact meaning
> and difference between open/closed status (is it only for the current
> commitfest?)

Closed means that the CF was in the past. It is archived now. Open
means that new patches are accepted to the given CF. If memory serves,
when the CF starts the status changes to "In Progress".

There are five CFs a year: in January, March, July, September, and
November. November one is about to start.

> and between «waiting for author» and «Returned with feedback».

RwF is almost the same as "Rejected". It means that some feedback was
provided for the patch and the community wouldn't mind accepting a new
patch when and if this feedback will be accounted for.

WfA means that the patch awaits some (relatively small) actions from
the author. Typically it happens after another round of code review.

Attached is a (!) simplified diagram of a typical patch livecycle.
Hopefully it will help a bit.

> I couldn't find a clear definition searching the wiki, the mailing list (too
> much unrelated results) or in the app itself.

Yes, this could be a tribe knowledge to a certain degree at the
moment. On the flip side this is also an opportunity to contribute an
article to the Wiki.

--
Best regards,
Aleksander Alekseev

Attachment

Re: Commitfest documentation

From
Jehan-Guillaume de Rorthais
Date:
Hi Aleksander,

Thank you for your help!

On Mon, 31 Oct 2022 16:51:23 +0300
Aleksander Alekseev <aleksander@timescale.com> wrote:
[...]
> > In the commitfest application, I was wondering today what was the exact
> > meaning and difference between open/closed status (is it only for the
> > current commitfest?)
>
> Closed means that the CF was in the past. It is archived now. Open
> means that new patches are accepted to the given CF. If memory serves,
> when the CF starts the status changes to "In Progress".

Sorry, I was asking from a patch point of view, not the whole commitfest. If
you look at the "Change Status" list on a patch page, there's two sublist
options: "Open statuses" and "Closed statuses". But your answer below answered
the question anyway.

> There are five CFs a year: in January, March, July, September, and
> November. November one is about to start.

This detail might have a place in the following page:
https://wiki.postgresql.org/wiki/CommitFest

But I'm not sure it's really worthy?

> > and between «waiting for author» and «Returned with feedback».
>
> RwF is almost the same as "Rejected". It means that some feedback was
> provided for the patch and the community wouldn't mind accepting a new
> patch when and if this feedback will be accounted for.
>
> WfA means that the patch awaits some (relatively small) actions from
> the author. Typically it happens after another round of code review.

Thank you for the disambiguation. Here is a proposal for all statuses:

 * Needs review: Wait for a new review.
 * WfA         : the patch awaits some (relatively small) actions from
                 the author, typically after another round of code review.
 * Ready fC    : No more comment from reviewer. The code is ready for a
                 commiter review.
 * Rejected    : The code is rejected. The community is not willing to accept
                 new patch about $subject.
 * Withdraw    : The author decide to remove its patch from the commit fest.
 * Returned wF : Some feedback was provided for the patch and the community
                 wouldn't mind accepting a new patch when and if this feedback
                 will be accounted for.
 * Move next CF: The patch is still waiting for the author, the reviewers or a
                 commiter at the end of the current CF.
 * Committed   : The patch as been committed.

> Attached is a (!) simplified diagram of a typical patch livecycle.
> Hopefully it will help a bit.

It misses a Withdraw box :)
I suppose it is linked from the Waiting on Author.

> > I couldn't find a clear definition searching the wiki, the mailing list (too
> > much unrelated results) or in the app itself.
>
> Yes, this could be a tribe knowledge to a certain degree at the
> moment. On the flip side this is also an opportunity to contribute an
> article to the Wiki.

I suppose these definitions might go in:
https://wiki.postgresql.org/wiki/Reviewing_a_Patch

However, I'm not strictly sure who is responsible to set these statuses. The
reviewer? The author? The commiter? The CF manager? I bet on the reviewer, but
it seems weird a random reviewer can reject a patch on its own behalf.

Regards,



Re: Commitfest documentation

From
Jacob Champion
Date:
On Mon, Oct 31, 2022 at 8:18 AM Jehan-Guillaume de Rorthais
<jgdr@dalibo.com> wrote:
> However, I'm not strictly sure who is responsible to set these statuses. The
> reviewer? The author? The commiter? The CF manager? I bet on the reviewer, but
> it seems weird a random reviewer can reject a patch on its own behalf.

Here's my current understanding (jump in and correct as needed):

Needs Review: If a patch is Waiting on Author, and then the author
responds to the requested feedback and would like additional review,
they can bump the patch back to this state. This can also be done by a
reviewer, or by the CFM, if the author forgets.

Waiting on Author: This is set by a reviewer when they believe a
response is necessary for the process to continue for a patch. Some
people set it immediately upon sending a request; others wait a few
days to keep the administrative overhead down. A CFM might put a patch
into this state if a reviewer forgets and the thread has been hanging
open for a while. (They should probably ping the thread at the same
time.)

Ready for Committer: A reviewer (or a CFM) puts a patch into this
state once they think the patchset is ready. Authors typically should
not put their own patches into this state unless there is already
general agreement on the list that it should be there.

Rejected: This status doesn't actually happen very often due to its
"final" nature. An individual reviewer should usually not decide this
unilaterally; propose rejection and wait for general agreement, or
wait for a CFM or a committer to come along.

Returned with Feedback: A CFM will typically set this at the end of a
CF. An author may preemptively do it as well, to "pause" review for
the entry while they work on it for a future CF.

Moved to Next CF: A CFM does this at the end of a CF, or an author
does it voluntarily.

Withdrawn: An author does this voluntarily to their own entry.

Committed: The committer or CFM does this.

--Jacob