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
signature.asc
Description: PGP signature
