Thread: Proposal: Make cfbot fail on patches not created by "git format-patch"
In the "Scaling PostgreSQL Development" unconference session. One of the problems that came up was that people don't follow "best practices". The response to that was that people don't know what the best practices are (nor that they are important to follow), because we don't enforce them. Based on the discussion there I'm planning to make the cfbot fail to apply a patch in the following two cases:
1. If a patch is not created by "git format-patch" (but cfbot will still use "patch" to apply the patch in case "git am" fails)
2. If the commit message has no body (so only a title)
Does anyone have strong opposition to this? To be clear, it means we don't run CI on patches created by piping "git diff" to a file anymore, as a way to nudge submitters into providing useful commit messages.
Communication wise, I plan to show this in the CF app as "Fails apply" (instead of "Needs Rebase"). When clicking the "Fails apply" link, it would then show a log as to why the apply failed. need to be created using git format patch, and should have a descriptive commit message.
Re: Proposal: Make cfbot fail on patches not created by "git format-patch"
From
Daniel Gustafsson
Date:
> On 16 May 2025, at 11:52, Jelte Fennema-Nio <me@jeltef.nl> wrote: > Does anyone have strong opposition to this? To be clear, it means we don't run CI on patches created by piping "git diff"to a file anymore, as a way to nudge submitters into providing useful commit messages. Disclaimer: I wasn't in the session (due to conflicting interesting sessions) so I might be raising points/questions already answered. Is this really lowering the bar for new contributors? I've always held "be liberal in what you accept" as a gold standard for projects I'm involved in, to remove barriers to entry. Good commit messages are obviously very important, but having your patch rejected (yes, I know, failing to apply) might not be strongest motivator for achieving this. -- Daniel Gustafsson
Jelte Fennema-Nio <me@jeltef.nl> writes:
> Based on the discussion there I'm planning to make the cfbot fail to apply
> a patch in the following two cases:
> ...
> To be clear, it means we don't
> run CI on patches created by piping "git diff" to a file anymore, as a way
> to nudge submitters into providing useful commit messages.
That outcome seems entirely horrible to me. If you want to flag the lack
of a commit message somehow, fine, but don't prevent CI from running.
regards, tom lane
Re: Proposal: Make cfbot fail on patches not created by "git format-patch"
From
Jacob Champion
Date:
On Fri, May 16, 2025 at 12:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > That outcome seems entirely horrible to me. If you want to flag the lack > of a commit message somehow, fine, but don't prevent CI from running. Personally I also prefer nudges to gates. Just like people already deprioritize "Waiting on Author" entries a bit, having an obvious "Patch Needs Work" note might gently help newcomers iterate on their first submissions (or even communicate where a patch is in the lifecycle! e.g. a Bugfix entry where the patch is marked incomplete might motivate someone to jump in to fix it). --Jacob
Jelte Fennema-Nio <me@jeltef.nl> writes:
> Okay, reasonable feedback. How about we add a CI job that does a
> "quality check". That's much less strong, as all the other tests will
> still run, but people would get a failing CI job which tells them that
> something is wrong. We could also include a pgindent in that CI job.
WFM
regards, tom lane