Documenting commitfest Rules - Mailing list pgsql-hackers

Hi Tomas, Everyone.


This seems like a good thing to discuss separately from the original
thread.

On 2018-03-02 22:06:32 +0100, Tomas Vondra wrote:
> Can you guys please point me to the CF rules that say this? Because my
> understanding (and not just mine, AFAICS) was obviously different.
> Clearly there's a disconnect somewhere.

I think we generally have documented the CF process very poorly.  Way
too much of it was discussed in-person at developer meetings (with notes
taken), is buried in deep threads arguing about release management or
individual commitfests.  A lot of the wiki information is outdated too.


Thus I'm trying to summarize my understanding in this email. It probably
doesn't 100% match other people's understanding. After we've resolved
those differences, we should update the wiki page, and delete outdated
content.


A single Commitfest is commonly called CF.

== Goal ==

Commitfests primarily exists so:

- we do not accidentally loose track of patch submissions that deserve
  attention, that previously happened frequently
- that reviewers and committers quickly can find patches deserving their
  attention (i.e. ones in in needs-review and ready-for-committer
  respectively)
- that there's a single point documenting the state of the patch, to
  avoid situations where different people interpret a thread differently
  without noticing.


== Entries ==

A commitfest entry is a patch, or set of patches, implementing some goal
(e.g. a new feature, some cleanup, improved docs). Each commitfest entry
is associated with one or more email threads, that discuss the feature.

A patch can be authored by multiple people, and the set of people
actively working on a patch can change.

Submitting a patch as a commitfest entry to a specific commitfest
implies a statement by the author that the patch needs input from
others. That input can be agreement on design decisions, high level code
review, testing, etc.

A CF entry can be in multiple states:

- Needs Review: this means that the patch author needs input to
  continue. While there may be some concurrent activity, the author will
  stall at some point without he input.

- Waiting for Author: there has been feedback to the author that makes
  continuing review unproductive. The most desirable case for that is
  because there has been a detailed review describing issues with the
  current implementation, which the patch author is expected to address
  or discuss.  Another reason might be that the current patch doesn't
  apply to the source tree anymore, and a rebased version of the patch
  is needed.

- Ready for Committer: reviewers have reviewed the patch and are,
  presumably after a few rounds of patch updates, happy with the current
  state and think the patch needs the attention of a committer. That can
  be because the patch is ready to commit, or because the design has
  evolved as far as it can without validation from somebody potentially
  willing to commit the patch.

- Committed: The patches associated with the CF entry have been
  committed.

- Rejected: The project has decided that the patch in the current form
  is not desirable.

- Returned with Feedback: The patch has gotten review indicating that it
  needs nontrivial changes, and the author did not have time and
  resources to perform them within the time of the current commitfest,
  or shortly thereafter.  It might be set because the author indicated
  so explicitly, stopped responding, etc.

- Moved to next CF: The patch was in needs review state, but either the
  current commitfest is about to end, or the patch is clearly in a state
  that is inappropriate for the current commitfest. The latter usually
  only matters in the last commitfest in a release cycle, more about
  that below.


== Commitfest Process ==

Commitfests start at a certain date. To avoid timezone issues, the
cutoff is when the start day has begun everywhere on the world.

All patches to be considered in that CF have to be submitted the day
before the start date, and either be in Needs Review or Ready for
Committer state.  That requirement exists because the goal of commitfest
is to provide input to patch authors so the patch can evolve and to not
forget patches that ought to be committed.

During a commitfest reviewers look at patches and provide feedback to
the authors. In the best case reviewers respond within a few days,
adapting the patch, answering changes, etc.

If the reviewers think a CF entry is ready for commit (or need design
input from a committer), the patch is set to Ready for Committer. Some
committer will hopefully come along and either commit the patch, or
explain what should happen in their opinion. Thus the followup states
can be Committed, Needs Review, Waiting on Author.

If patches are in Waiting for Author state for a while, after a
"warning" by the CF manager or reviewers, the patch will be marked as
Returned with Feedback, signaling that the project is not expected to
continue working on the patch.  If a patch needs extensive changes
(i.e. it's unrealistic they are made before the end of the current CF)
it often also can make sense to mark such patches as Returned with
Feedback, but input by the authors should be sought.

If patches are in Needs Review state at the end of the commitfest, the
patch will be moved to the next commitfest. If they are Waiting on
Author, the patch should be marked as Returned with Feedback.


== Last Commitfest ==

The last commitfest is the CF before the expected feature freeze date
for the current yearly postgres release.

A few extra restrictions are in place, to make it more likely to finish
features that have a realistic chance of ending up in the current
release.

No nontrivial patches that have not previously been submitted to an
earlier CF should be entered into the CF. Otherwise new and shiny
patches will take up all the bandwidth from patches that have been
worked on for a long time.

Additionally patches that cannot realistically be finished within the
last commitfest will aggressively be moved to the next commitfest early,
to avoid using up resources that should be used to finish features for
the current release.


== Commitfest Manager ==

The commitfest's manager is determined at the beginning of the
commitfest. Sometimes multiple people will serve that role for a single
CF, to share the load.

Their role is to attempt to ensure that patches in the commitfest are in
an appropriate state, to bring up when that's not the case, and to
attempt to get all patches a fair amount of attention.



What do others think, is this a reasonable description of the *current*
process? Where do you think it is inaccurate?  I'd appreciate if we
could discuss *improvements* to the process separately. Let's first
agree on where we are.


Greetings,

Andres Freund