| Message ID | 20201228004152.522421-1-seth@eseth.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mergetool: add automerge configuration | expand |
Seth House <seth@eseth.com> writes: > * Signed off on Felipe's commit. (Although I have minor qualms with > Felipe's various wording and even the name of the flag it is > decidedly not worth burdening the list with bike-shedding.) Even when the original is a horrible patch in your opinion that is laden with bugs, as long as the original author signed it off (which means that the original author certifies that it can be included in and distributed by the project under our licensing terms, and agrees to the fact that the original author did so will be recorded in perpetuity), you can relay such a patch as-is, and you are required (i.e. SubmittingPatches is pretty clear that without your sign-off we cannot accept) to sign it off to record the provenance of the code. The other side of the above coin is that you are not endorsing or vounching for the patch when you sign it off, so your name is not smudged by wording and flag name chosen in a way that you may consider poor. So "Although..." part is not a good objection against signing it off. In other words, sign-off is not about assuring quality. Also, instead of relaying as-is, you can relay a patch with your improvements rolled into the same patch (i.e. not as follow-up fixes). Some (or major) parts of the original patch may still remain in the edited result and you'd need to keep original author's sign-off as-is [*1*]. In this topic's case, 2/2 would be a feature enhancement on top of 1/2, so relaying 1/2 as-is would be OK, but in a case where an promising patch was sent with sign-off and bugs, then gets abandoned by the original author, fixing the bug in the patch you relay in place (i.e. not as follow-up patches) may even be necessary to keep bisectability. When you do so, you'd typically do: Subject: [PATCH] title of the patch ... original author's log message, possibly copyedited ... by you + <Comment on what you did on top of the original can come here> Signed-off-by: Original Author <ori@ginal.au.thor> + [or brief comment here] + Signed-off-by: Your Name <you@your.do.main> (1) add your sign-off at the end (2) explain what you changed relative to the original, either inside [] on the line before your sign-off, or at the end of the log message proper. to indicate that it is not relayed as-is; this allows you to take responsibility of an unintended breakage your "fixes" might have caused. [Footnote] *1* The result may become something that no longer aligns the original author's opinion, but that is OK. The sign-off by the original author just says that the original author has the right to contribute (the remaining part of) the patch and the original author agrees that the record of author's involvement in the patch (including sign-off) will be kept. It is not about assuring quality of the final work by the original author, either.
Junio C Hamano wrote: > Also, instead of relaying as-is, you can relay a patch with your > improvements rolled into the same patch (i.e. not as follow-up > fixes). Some (or major) parts of the original patch may still > remain in the edited result and you'd need to keep original author's > sign-off as-is [*1*]. Yes, you *can*, but doing so in this case would be against the author's wishes, and a violation of the Developer Certificate of Origin. > In this topic's case, 2/2 would be a feature enhancement on top of > 1/2, so relaying 1/2 as-is would be OK, but in a case where an > promising patch was sent with sign-off and bugs, then gets abandoned > by the original author, fixing the bug in the patch you relay in > place (i.e. not as follow-up patches) may even be necessary to keep > bisectability. When you do so, you'd typically do: If the author doesn't object (which is usually the case), this makes sense. But if the author objects, you would be violating clause (d) of the DCO. Just because the only way to do X is to violate laws, terms, or agreements doesn't mean that's what you should do. You can simply not do X.
Changes since v6: * Incorporated Junio's help and advice: * Rebased v7 off of the correct v5 version. * Signed off on Felipe's commit. (Although I have minor qualms with Felipe's various wording and even the name of the flag it is decidedly not worth burdening the list with bike-shedding.) Changes since v5: * Add per-tool configuration that Felipe has a "deep philosophical" opposition to adding. Felipe Contreras (1): mergetool: add automerge configuration Seth House (1): mergetool: Add per-tool support for the autoMerge flag Documentation/config/mergetool.txt | 6 ++++++ git-mergetool.sh | 20 ++++++++++++++++++++ t/t7610-mergetool.sh | 18 ++++++++++++++++++ 3 files changed, 44 insertions(+)