* Skipping adding Signed-off-by even if it's not the last on git commit @ 2022-12-06 17:06 David Caro 2022-12-07 1:50 ` Taylor Blau 0 siblings, 1 reply; 14+ messages in thread From: David Caro @ 2022-12-06 17:06 UTC (permalink / raw) To: git [-- Attachment #1: Type: text/plain, Size: 856 bytes --] Hi! I have noticed that when requesting git commit to add the Signed-off-by header, it will add it even if it already exists as long as it's not the last in the footer. It seems that the functionality to do so has been there for a while (see [1]) and it's being used effectively on format-patch, but it was never enabled for the rest of commands. I propose enabling it for commit, merge and am. I can easily send a patch for it if the change is welcome. Thanks! [1] https://github.com/git/git/commit/bab4d1097c8be7d688a53b992232063dbf300fd4 -- David Caro SRE - Cloud Services Wikimedia Foundation <https://wikimediafoundation.org/> PGP Signature: 7180 83A2 AC8B 314F B4CE 1171 4071 C7E1 D262 69C3 "Imagine a world in which every single human being can freely share in the sum of all knowledge. That's our commitment." [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-06 17:06 Skipping adding Signed-off-by even if it's not the last on git commit David Caro @ 2022-12-07 1:50 ` Taylor Blau 2022-12-07 4:11 ` Junio C Hamano 2022-12-07 4:31 ` Ævar Arnfjörð Bjarmason 0 siblings, 2 replies; 14+ messages in thread From: Taylor Blau @ 2022-12-07 1:50 UTC (permalink / raw) To: David Caro; +Cc: git Hi David, On Tue, Dec 06, 2022 at 06:06:46PM +0100, David Caro wrote: > I have noticed that when requesting git commit to add the > Signed-off-by header, it will add it even if it already exists as long > as it's not the last in the footer. > > It seems that the functionality to do so has been there for a while > (see [1]) and it's being used effectively on format-patch, but it was > never enabled for the rest of commands. Right; bab4d1097c (sequencer.c: teach append_signoff how to detect duplicate s-o-b, 2013-02-12) introduced the APPEND_SIGNOFF_DEDUP flag, which instructs append_signoff() to avoid adding a Signed-off-by trailer if one already exists anywhere in the trailers section of the target commit. As bab4d1097c hints, this is desirable in some situations. However, adding duplicated S-o-b trailers is useful in other situations. For example, if you and I exchange a patch back and forth on this list, it might go something like: 1. I write up an initial patch, add my Singed-off-by, and send it to the list. 2. You notice that some aspects can be improved, so you apply and modify the commit locally, adding your own Signed-off-by. 3. I pick up your changes as part of a new version of the patch series, and resubmit it to the list, adding my own Signed-off-by again. There are a couple of small things worth noting there. First, in (2), the changes that you introduced were large enough to be worth crediting (e.g., you didn't suggest a typofix or to modify the amount of spacing on a line or similar), but not large enough to change the author of the patch. Likewise, in (3), the second instance of my Signed-off-by indicates that I am OK with the changes that you wrote, and I certify the new version of the patch as my own work. One such instance is in 0ca6ead81e (alias.c: reject too-long cmdline strings in split_cmdline(), 2022-09-28). Another is in b3c5f5cb04 (submodule: move core cmd_update() logic to C, 2022-03-15). If you're curious to dig up more in your own project, here's a little shell snippet I wrote up to find some examples: git rev-list --no-merges HEAD | while read rev do git -P show -s -1 --format='%(trailers:key=Signed-off-by,unfold=true)' "$rev" | grep -v '^$' | sort | uniq -c | grep -vq "^ 1 " && echo "$rev" done > I propose enabling it for commit, merge and am. So I think that there are some legitimate uses outside of 'format-patch' that we may want to keep the existing behavior. So I don't think we should necessarily change the default to have other commands outside of 'format-patch' start passing APPEND_SIGNOFF_DEDUP. But I could see a future where we add a new option that controls whether or not we pass that flag, perhaps: $ git commit --signoff[=[no-]dedup] ? Thanks, Taylor ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 1:50 ` Taylor Blau @ 2022-12-07 4:11 ` Junio C Hamano 2022-12-07 8:40 ` David Caro 2022-12-07 4:31 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2022-12-07 4:11 UTC (permalink / raw) To: Taylor Blau; +Cc: David Caro, git Taylor Blau <me@ttaylorr.com> writes: >> I propose enabling it for commit, merge and am. > > So I think that there are some legitimate uses outside of 'format-patch' > that we may want to keep the existing behavior. So I don't think we > should necessarily change the default to have other commands outside of > 'format-patch' start passing APPEND_SIGNOFF_DEDUP. > > But I could see a future where we add a new option that controls whether > or not we pass that flag, perhaps: > > $ git commit --signoff[=[no-]dedup] That sounds sensible. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 4:11 ` Junio C Hamano @ 2022-12-07 8:40 ` David Caro 2022-12-07 22:13 ` Taylor Blau 0 siblings, 1 reply; 14+ messages in thread From: David Caro @ 2022-12-07 8:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Blau, git [-- Attachment #1: Type: text/plain, Size: 984 bytes --] On 12/07 13:11, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > >> I propose enabling it for commit, merge and am. > > > > So I think that there are some legitimate uses outside of 'format-patch' > > that we may want to keep the existing behavior. So I don't think we > > should necessarily change the default to have other commands outside of > > 'format-patch' start passing APPEND_SIGNOFF_DEDUP. > > > > But I could see a future where we add a new option that controls whether > > or not we pass that flag, perhaps: > > > > $ git commit --signoff[=[no-]dedup] > > That sounds sensible. Agree, I will implement this then for commit for now, thanks! -- David Caro SRE - Cloud Services Wikimedia Foundation <https://wikimediafoundation.org/> PGP Signature: 7180 83A2 AC8B 314F B4CE 1171 4071 C7E1 D262 69C3 "Imagine a world in which every single human being can freely share in the sum of all knowledge. That's our commitment." [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 8:40 ` David Caro @ 2022-12-07 22:13 ` Taylor Blau 2022-12-07 23:04 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Taylor Blau @ 2022-12-07 22:13 UTC (permalink / raw) To: David Caro; +Cc: Junio C Hamano, git On Wed, Dec 07, 2022 at 09:40:27AM +0100, David Caro wrote: > On 12/07 13:11, Junio C Hamano wrote: > > Taylor Blau <me@ttaylorr.com> writes: > > > > >> I propose enabling it for commit, merge and am. > > > > > > So I think that there are some legitimate uses outside of 'format-patch' > > > that we may want to keep the existing behavior. So I don't think we > > > should necessarily change the default to have other commands outside of > > > 'format-patch' start passing APPEND_SIGNOFF_DEDUP. > > > > > > But I could see a future where we add a new option that controls whether > > > or not we pass that flag, perhaps: > > > > > > $ git commit --signoff[=[no-]dedup] > > > > That sounds sensible. > > Agree, I will implement this then for commit for now, thanks! Thanks, I look forward to seeing your work. It would be nice to standardize on this `--signoff[=[no-]dedup]` thing throughout all of the different commands that support it. According to: $ git grep -- '--signoff' -- Documentation/git-*.txt that list is: `git am`, `git cherry-pick`, `git format-patch`, `git rebase`, and `git revert`. But there are others, too, via an include of `signoff-option.txt`, so that is `git commit`, `git merge`, and `git pull`. Phew ;-). Thanks, Taylor ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 22:13 ` Taylor Blau @ 2022-12-07 23:04 ` Junio C Hamano 2022-12-08 7:27 ` Jeff King 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2022-12-07 23:04 UTC (permalink / raw) To: Taylor Blau; +Cc: David Caro, git Taylor Blau <me@ttaylorr.com> writes: >> > > $ git commit --signoff[=[no-]dedup] > ... > Thanks, I look forward to seeing your work. It would be nice to > standardize on this `--signoff[=[no-]dedup]` thing throughout all of the > different commands that support it. Also, if I am not mistaken, each of trailers can be configured to have its own semantics (e.g. .where and .ifExists). * Should we have similar override to these trailer tokens, not just sign-off? * Should we offer not just [no-]dedup (which is equivalent to switching from addIfDifferentNeighbor to addIfDifferent) but other customization? This affects what --signoff should be allowed to take for consistency across trailers. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 23:04 ` Junio C Hamano @ 2022-12-08 7:27 ` Jeff King 2022-12-09 1:23 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Jeff King @ 2022-12-08 7:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Blau, David Caro, git On Thu, Dec 08, 2022 at 08:04:46AM +0900, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > >> > > $ git commit --signoff[=[no-]dedup] > > ... > > Thanks, I look forward to seeing your work. It would be nice to > > standardize on this `--signoff[=[no-]dedup]` thing throughout all of the > > different commands that support it. > > Also, if I am not mistaken, each of trailers can be configured to > have its own semantics (e.g. .where and .ifExists). > > * Should we have similar override to these trailer tokens, not just > sign-off? This made me curious about the opposite: is there config you can set to get this behavior for --signoff? I think the answer is "no". You can do: git -c trailer.ifExists=addIfDifferent \ commit --amend --trailer="Signed-off-by: Jeff King <peff@peff.net>" to get the desired behavior, but using "--signoff" does not respect trailer settings. I feel like config is probably a better match for the use cases here, because the decision about de-duping is not something you'd usually set for one particular operation, but is more likely to be a project policy about what the trailer means (and that includes Signed-off-by). So you'd want to set it per-repo, not per-operation. > * Should we offer not just [no-]dedup (which is equivalent to > switching from addIfDifferentNeighbor to addIfDifferent) but > other customization? This affects what --signoff should be > allowed to take for consistency across trailers. Yeah, writing the above made me wonder if --signoff should behave exactly like: --trailer="Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" For the most part that would not change any behavior unless you set trailer.signed-off-by.*, and setting those presumably signals intent that you expect them to take effect. The one exception is that the generic trailer.ifExists, etc, would start affecting --signoff, which _might_ be a surprise. If we wanted to retain the behavior there, we could say "--signoff is special, and doesn't respect generic trailer config". Alternatively, it would be nice if there was an easy way to put your ident into a trailer (what I wrote above doesn't really work unless you have those variables in your environment, and of course it's a lot of typing). I think you can hack it up like: git config trailer.sign.key Signed-off-by git config trailer.sign.cmd \ 'git var GIT_COMMITTER_IDENT | sed "s/>.*/>/";:' git commit --trailer=sign which is only a little more typing than --signoff, but it's not very ergonomic. -Peff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-08 7:27 ` Jeff King @ 2022-12-09 1:23 ` Junio C Hamano 2022-12-09 1:42 ` Jeff King 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2022-12-09 1:23 UTC (permalink / raw) To: Jeff King; +Cc: Taylor Blau, David Caro, git Jeff King <peff@peff.net> writes: > This made me curious about the opposite: is there config you can set to > get this behavior for --signoff? I think the answer is "no". You can do: > > git -c trailer.ifExists=addIfDifferent \ > commit --amend --trailer="Signed-off-by: Jeff King <peff@peff.net>" > > to get the desired behavior, but using "--signoff" does not respect > trailer settings. A customization to allow addIfDifferent may be an interesting idea. > I feel like config is probably a better match for the use cases here, > because the decision about de-duping is not something you'd usually set > for one particular operation, but is more likely to be a project policy > about what the trailer means (and that includes Signed-off-by). So you'd > want to set it per-repo, not per-operation. Sure. > ... The one exception is that the > generic trailer.ifExists, etc, would start affecting --signoff, which > _might_ be a surprise. If we wanted to retain the behavior there, we > could say "--signoff is special, and doesn't respect generic trailer > config". Yeah, that may be safe, however it is very unsatisfying. > Alternatively, it would be nice if there was an easy way to put your > ident into a trailer (what I wrote above doesn't really work unless you > have those variables in your environment, and of course it's a lot of > typing). I think you can hack it up like: > > git config trailer.sign.key Signed-off-by > git config trailer.sign.cmd \ > 'git var GIT_COMMITTER_IDENT | sed "s/>.*/>/";:' > git commit --trailer=sign > > which is only a little more typing than --signoff, but it's not very > ergonomic. It does not look _too_ bad, though. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-09 1:23 ` Junio C Hamano @ 2022-12-09 1:42 ` Jeff King 2022-12-09 4:03 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Jeff King @ 2022-12-09 1:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Blau, David Caro, git On Fri, Dec 09, 2022 at 10:23:52AM +0900, Junio C Hamano wrote: > > ... The one exception is that the > > generic trailer.ifExists, etc, would start affecting --signoff, which > > _might_ be a surprise. If we wanted to retain the behavior there, we > > could say "--signoff is special, and doesn't respect generic trailer > > config". > > Yeah, that may be safe, however it is very unsatisfying. Agreed. ;) > > Alternatively, it would be nice if there was an easy way to put your > > ident into a trailer (what I wrote above doesn't really work unless you > > have those variables in your environment, and of course it's a lot of > > typing). I think you can hack it up like: > > > > git config trailer.sign.key Signed-off-by > > git config trailer.sign.cmd \ > > 'git var GIT_COMMITTER_IDENT | sed "s/>.*/>/";:' > > git commit --trailer=sign > > > > which is only a little more typing than --signoff, but it's not very > > ergonomic. > > It does not look _too_ bad, though. What I don't like about it is: - the external cmd is complicated and slow. It would be nice if you could just set trailer.sign.ident=true or something, and it would use your ident by default if no value is given (and maybe even do completion similar to "commit --author" if a value is given). - you have to know to be clever enough to define and use --trailer=sign. If --signoff didn't exist, that's not too big a stretch. But since it does, everyone will naturally reach for it first. The two could be solved independently, of course. And even if we did want to solve both by making --signoff behave more like other trailers, having trailer.*.ident might be nice in general. I admit I just thought of this concept, though, so I don't have plans to work up patches, and I make no claims there isn't a gotcha waiting somewhere. ;) -Peff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-09 1:42 ` Jeff King @ 2022-12-09 4:03 ` Junio C Hamano 2022-12-09 20:39 ` Jeff King 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2022-12-09 4:03 UTC (permalink / raw) To: Jeff King; +Cc: Taylor Blau, David Caro, git Jeff King <peff@peff.net> writes: >> > git config trailer.sign.key Signed-off-by >> > git config trailer.sign.cmd \ >> > 'git var GIT_COMMITTER_IDENT | sed "s/>.*/>/";:' >> > git commit --trailer=sign >> > >> > which is only a little more typing than --signoff, but it's not very >> > ergonomic. >> >> It does not look _too_ bad, though. > > What I don't like about it is: > > - the external cmd is complicated and slow. It would be nice if you > could just set trailer.sign.ident=true or something, and it would > use your ident by default if no value is given (and maybe even do > completion similar to "commit --author" if a value is given). Ah, "trailer.sign.value" to use the same value does not exist, and the closest kludge we can use is the .cmd thing? Then it is a shame but it should be easy to correct? > - you have to know to be clever enough to define and use > --trailer=sign. If --signoff didn't exist, that's not too big a > stretch. But since it does, everyone will naturally reach for it > first. We could reimplement --signoff to actually take attention to the "trailer.sign.*" thing, if we wanted to, and that makes it very easy to explain, I guess. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-09 4:03 ` Junio C Hamano @ 2022-12-09 20:39 ` Jeff King 0 siblings, 0 replies; 14+ messages in thread From: Jeff King @ 2022-12-09 20:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Blau, David Caro, git On Fri, Dec 09, 2022 at 01:03:44PM +0900, Junio C Hamano wrote: > > What I don't like about it is: > > > > - the external cmd is complicated and slow. It would be nice if you > > could just set trailer.sign.ident=true or something, and it would > > use your ident by default if no value is given (and maybe even do > > completion similar to "commit --author" if a value is given). > > Ah, "trailer.sign.value" to use the same value does not exist, and > the closest kludge we can use is the .cmd thing? Then it is a shame > but it should be easy to correct? Yeah, I almost suggested trailer.sign.value at first, but on further thought, it would be kind of weird. When the user asks for a trailer to be included, it usually already has a value along with it. This could be thought of as a default value, which might work like: git config trailer.signed-off-by.default "Jeff King <peff@peff.net>" That's pretty flexible, but it seems like what you really want is not some string, but to say "my usual committer ident". And so we force you to repeat what's in user.email, etc, or we have to invent some new token that means "my usual ident". So if we instead have a flag for "this is an ident", then it can just do the expected thing. > > - you have to know to be clever enough to define and use > > --trailer=sign. If --signoff didn't exist, that's not too big a > > stretch. But since it does, everyone will naturally reach for it > > first. > > We could reimplement --signoff to actually take attention to the > "trailer.sign.*" thing, if we wanted to, and that makes it very easy > to explain, I guess. Yeah. My only reservation would be whether anybody would be surprised by the change in behavior. I have the feeling that hardly anybody uses trailer.* config, but I admit that's not based in any real data. -Peff ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 1:50 ` Taylor Blau 2022-12-07 4:11 ` Junio C Hamano @ 2022-12-07 4:31 ` Ævar Arnfjörð Bjarmason 2022-12-07 4:57 ` Junio C Hamano 2022-12-07 15:00 ` Konstantin Ryabitsev 1 sibling, 2 replies; 14+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-12-07 4:31 UTC (permalink / raw) To: Taylor Blau; +Cc: David Caro, git On Tue, Dec 06 2022, Taylor Blau wrote: > Hi David, > > On Tue, Dec 06, 2022 at 06:06:46PM +0100, David Caro wrote: >> I have noticed that when requesting git commit to add the >> Signed-off-by header, it will add it even if it already exists as long >> as it's not the last in the footer. >> >> It seems that the functionality to do so has been there for a while >> (see [1]) and it's being used effectively on format-patch, but it was >> never enabled for the rest of commands. > > Right; bab4d1097c (sequencer.c: teach append_signoff how to detect > duplicate s-o-b, 2013-02-12) introduced the APPEND_SIGNOFF_DEDUP flag, > which instructs append_signoff() to avoid adding a Signed-off-by > trailer if one already exists anywhere in the trailers section of the > target commit. > > As bab4d1097c hints, this is desirable in some situations. However, > adding duplicated S-o-b trailers is useful in other situations. For > example, if you and I exchange a patch back and forth on this list, it > might go something like: > > 1. I write up an initial patch, add my Singed-off-by, and send it to > the list. > > 2. You notice that some aspects can be improved, so you apply and > modify the commit locally, adding your own Signed-off-by. > > 3. I pick up your changes as part of a new version of the patch > series, and resubmit it to the list, adding my own Signed-off-by > again. > > There are a couple of small things worth noting there. First, in (2), > the changes that you introduced were large enough to be worth crediting > (e.g., you didn't suggest a typofix or to modify the amount of spacing > on a line or similar), but not large enough to change the author of the > patch. > > Likewise, in (3), the second instance of my Signed-off-by indicates that > I am OK with the changes that you wrote, and I certify the new version > of the patch as my own work. > > One such instance is in 0ca6ead81e (alias.c: reject too-long cmdline > strings in split_cmdline(), 2022-09-28). Another is in b3c5f5cb04 > (submodule: move core cmd_update() logic to C, 2022-03-15). > > If you're curious to dig up more in your own project, here's a little > shell snippet I wrote up to find some examples: > > git rev-list --no-merges HEAD | > while read rev > do > git -P show -s -1 --format='%(trailers:key=Signed-off-by,unfold=true)' "$rev" | > grep -v '^$' | sort | uniq -c | grep -vq "^ 1 " && echo "$rev" > done This is pretty much what I would have said, i.e. that it is intentional, as the SOB has do do with the DCO chronology of events. The SOB is a proxy for passing the copyrightable work around, and to certify that you have permission to license the work per the DCO etc. But it's also interesting in that context that we choose to omit this for re-rolls. I.e.: 1. I write a patch, that has 10 lines of original work, add a SOB 2. You pick it up, add another 10 lines, add your SOB 3. I pick it up again, add another 10 lines, add my SOB So at the end we have a SOB sequence of: Ævar, Taylor, Ævar, and 30 copyrightable lines of code. But if I submit three versions of my own patch with the same growth pattern over those three iterations shouldn't I have 3 of my own SOB lines: Ævar, Ævar, Ævar? I'm not suggesting that's a sane idea, but at the same time I can't really square the internal logic of why we'd do one, but not the other. If we assume that an earlier SOB from a v1 is carried forward to my own v2 and v3 re-rolls, shouldn't the same convention apply for my v1 SOB if you the v2 re-roll was yours? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 4:31 ` Ævar Arnfjörð Bjarmason @ 2022-12-07 4:57 ` Junio C Hamano 2022-12-07 15:00 ` Konstantin Ryabitsev 1 sibling, 0 replies; 14+ messages in thread From: Junio C Hamano @ 2022-12-07 4:57 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Taylor Blau, David Caro, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > But if I submit three versions of my own patch with the same growth > pattern over those three iterations shouldn't I have 3 of my own SOB > lines: Ævar, Ævar, Ævar? The difference, I think, is that it is not worth recording if it is just you regurgitated the same idea a few times yourself without input from others. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Skipping adding Signed-off-by even if it's not the last on git commit 2022-12-07 4:31 ` Ævar Arnfjörð Bjarmason 2022-12-07 4:57 ` Junio C Hamano @ 2022-12-07 15:00 ` Konstantin Ryabitsev 1 sibling, 0 replies; 14+ messages in thread From: Konstantin Ryabitsev @ 2022-12-07 15:00 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Taylor Blau, David Caro, git On Wed, Dec 07, 2022 at 05:31:18AM +0100, Ævar Arnfjörð Bjarmason wrote: > The SOB is a proxy for passing the copyrightable work around, and to > certify that you have permission to license the work per the DCO etc. > > But it's also interesting in that context that we choose to omit this > for re-rolls. I.e.: > > 1. I write a patch, that has 10 lines of original work, add a SOB > 2. You pick it up, add another 10 lines, add your SOB > 3. I pick it up again, add another 10 lines, add my SOB > > So at the end we have a SOB sequence of: Ævar, Taylor, Ævar, and 30 > copyrightable lines of code. > > But if I submit three versions of my own patch with the same growth > pattern over those three iterations shouldn't I have 3 of my own SOB > lines: Ævar, Ævar, Ævar? I'm not sure the trailer order matters much for the copyright info, because after all this information is in the output of "git blame" and "git log". That said, "trailer order" is a touchy subject with kernel developers, because it means different things to pretty much every kernel maintainer. With b4, I'm sticking to the "chain of custody" (COC) approach, which treats "S-o-b" trailers as markers for where the chain of custody boundaries are. For example, consider the following patch: | [PATCH] foo: implement libbar | | This patch implements libbar. | | Suggested-by: Reporter 1 <...> | Link: https://msgid.link/some-msgid | Signed-off-by: Developer 1 <...> -- initial COC boundary | Reviewed-by: Reviewer 1 <...> | Tested-by: Tester 1 <...> | Signed-off-by: Submaintainer 1 <...> -- intermediate COC boundary | Acked-by: Submaintainer 2 <...> | Signed-off-by: Maintainer 1 <...> -- final COC boundary In terms of COC, this patch makes the following claims: Developer 1: - I am responsible for this code - It was suggested by Reporter 1 - You can read about it at this URL Submaintainer 1: - I am signing off on this code - I am the one who added the trailers from Reviewer 1 and Tester 1 Maintainer 1: - I am signing off on this code - I am the one who added the trailer from Submaintainer 2 In the chain-of-custody scheme multiple identical trailers don't make sense, as far as I can tell. If the patch doesn't pass review and someone returns it back to the original developer, their "S-o-b: Developer 1" trailer simply moves down to be the last entry in v2 of the patch, and the developer should remove any code review trailers that were issued for the previous version of the patch. This is really the only order that makes sense to me. :) -K ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-12-09 20:40 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-06 17:06 Skipping adding Signed-off-by even if it's not the last on git commit David Caro 2022-12-07 1:50 ` Taylor Blau 2022-12-07 4:11 ` Junio C Hamano 2022-12-07 8:40 ` David Caro 2022-12-07 22:13 ` Taylor Blau 2022-12-07 23:04 ` Junio C Hamano 2022-12-08 7:27 ` Jeff King 2022-12-09 1:23 ` Junio C Hamano 2022-12-09 1:42 ` Jeff King 2022-12-09 4:03 ` Junio C Hamano 2022-12-09 20:39 ` Jeff King 2022-12-07 4:31 ` Ævar Arnfjörð Bjarmason 2022-12-07 4:57 ` Junio C Hamano 2022-12-07 15:00 ` Konstantin Ryabitsev
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).