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
