On Tue, Jan 16, 2018 at 04:51:13PM -0300, Alvaro Herrera wrote:
> David Fetter wrote:
>
>> I'm sure I'm not alone in finding it helpful when patch sets come with
>> a single-sentence summary of the patch set and a commit message for
>> each individual patch.
>>
>> Is git format-patch really too heavy a lift to ask of people?
>
> I think it's okay as general guideline, but not as a hard requirement.
> Just like we advise people to trim quoted text when they reply to
> mailing list postings, but we don't boot those that fail to.
I agree with this position. Sometimes even patches created with
format-patch fail to apply with git apply after rotting a bit (because
git apply/am also complains about offsets more easily? And cherry-pick
forgives more easily?). At the end I generally finish by applying things
with patch -p1 after testing with git apply/am. As a general guideline,
if a patch can be applied cleanly with patch -p1, then the thing should
not need a rebase. There could be issues with misplaced blocks because
of offsets, those usually finish with compilation failures, not usually
with regression test failures. If those happen requesting a rebase is
fine, but like Robert there is no point to complain about a patch that
applies and works with offsets. Mentioning that is good because that's a
sign that a patch is aging, but that's not an argument sufficient for a
rebase.
Some communities have hard guidelines for patch format with their patch
submission, which tend to make people refrain to contribute even small
patches (which get ignored by upstream committers at the end), as the
set of basic guidelines is harder to learn than producing a simple
patch. Personally as long as I can read a patch proposed for a bug fix
from a text file, on which I am able to understand the intention behind,
then that's acceptable to dive into as the goal is to fix an existing
problem. Patches for features able to apply are fine to look at (of
course this depends on the feature, the docs it has, what the
contributor is proposing, etc).
An idea could be to add more detailed guidelines in the wiki for the
patch review section:
https://wiki.postgresql.org/wiki/Submitting_a_Patch
Perhaps something among the lines: "When a feature requires deep
surgery, dividing a patch into several entries with git-format-patch
with a proper commit log is recommended and eases review, though this is
not mandatory. git apply/am are very picky commands, so as long as a
patch can apply with patch -p1 consider yourself covered."
--
Michael
