Re: [PATCH RFC] git-am: support any number of signatures
From: Junio C Hamano > > Christian Couder writes: > >> On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder >> wrote: >>> >>> With v16 you can easily choose if you want to have the S-o-b in the >>> output or not, when there is already one, ... >> >> By the way, I sent v16 just before the above email, but the series >> still hasn't hit the mailing list. >> Did some of you guys in cc receive something? > > I see them and picked them up to replace. Thanks! > Are these now ready for 'next'? Yeah, I think so. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Christian Couder writes: > On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder > wrote: >> >> With v16 you can easily choose if you want to have the S-o-b in the >> output or not, when there is already one, ... > > By the way, I sent v16 just before the above email, but the series > still hasn't hit the mailing list. > Did some of you guys in cc receive something? I see them and picked them up to replace. Are these now ready for 'next'? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder wrote: > > With v16 you can easily choose if you want to have the S-o-b in the > output or not, when there is already one, ... By the way, I sent v16 just before the above email, but the series still hasn't hit the mailing list. Did some of you guys in cc receive something? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Sun, Sep 28, 2014 at 9:32 PM, Junio C Hamano wrote: > Christian Couder writes: > >> From: Junio C Hamano >> >>> If that is what happens, it is not a workable workaround to set Sob to >>> addIfDifferent only for this invocation. >> >> Setting S-o-b to addIfDifferent for this invocation would not add the >> S-o-b at the end, because another S-o-b still exists in the input >> message "seen" when the last S-o-b is processed. > > So there is no workaround whatsoever, which is worse X-<. I think there might be a misunderstanding. With v16 you can easily choose if you want to have the S-o-b in the output or not, when there is already one, see: $ cat test.txt Signed-off-by: $ cat test.txt | git interpret-trailers --trailer 'Acked-by: ' --trailer 'Reviewed-by: ' --trailer 'Tested-by: ' --trailer 'Signed-off-by: ' Signed-off-by: Acked-by: Reviewed-by: Tested-by: Signed-off-by: $ cat test.txt | git -c 'trailer.ifexists=addIfDifferent' interpret-trailers --trailer 'Acked-by: ' --trailer 'Reviewed-by: ' --trailer 'Tested-by: ' --trailer 'Signed-off-by: ' Signed-off-by: Acked-by: Reviewed-by: Tested-by: Or: $ cat test.txt | git -c 'trailer.Signed-off-by.ifexists=addIfDifferent' interpret-trailers --trailer 'Acked-by: ' --trailer 'Reviewed-by: ' --trailer 'Tested-by: ' --trailer 'Signed-off-by: ' Signed-off-by: Acked-by: Reviewed-by: Tested-by: (There was a small bug in v15 with the last command above.) >>> Alternatively, if Neighbor-ness is evaluated first before you add A/R/T >>> in response to this request, then you'd refrain from adding a duplicate >>> Sob. It wasn't quite clear from your description what your design was, >>> and your explanation above is not still clear, at least to me. >> >> I hope it is clearer now. Maybe I should add something in the doc to >> better explain how it works. > > I doubt that it would help the users materially to document that we > chose to implement a less useful way when there are multiple ways in > which a feature can work, though. > > Unless I am mis-reading you and you are actually saying that the > users can emulate the "atomic" variant without much hassle by doing > X and Y, that is. If so, it would help readers to document them. If you would like me to document the 3 above commands in an example, I am ok to do that. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Jeff King writes: > Optional parameters for arguments make backwards-compatibility tricky. > In this case, the command: > > git am -s mbox1 mbox2 > > means "apply the patches from mbox1 and mbox2, and signoff the patches". > Under your scheme, it now means "apply from mbox2, and use the trailer > mbox1". Thanks for saving me from typing ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Wed, Oct 08, 2014 at 12:29:37AM +0300, Michael S. Tsirkin wrote: > > If I understand it correctly, Michael is envisioning to implement > > his "git am -s art" (I would recommend against reusing -s for this, > > though. "git am --trailer art" is fine) and doing so by using > > interpret-trailers as an internal implementation detail, so I would > > say the above is a perfectly fine way to do so. An equivalent of > > that command line is synthesized and run internally in his version > > of "git am" when his "git am" sees "--trailer art" option using > > those am.{"a","r","t"}.trailer configuration variables. > > Hmm I wonder why do you dislike reusing -s with a parameter for this. > To me, this looks like a superset of the default -s functionality: -s > adds the default signature, -s "x" adds signature "x" ... Users don't > really care that one is implemented as a trailer and another isn't. In > fact, default -s can be implemented as a trailer too, right? > > Could you clarify please? Optional parameters for arguments make backwards-compatibility tricky. In this case, the command: git am -s mbox1 mbox2 means "apply the patches from mbox1 and mbox2, and signoff the patches". Under your scheme, it now means "apply from mbox2, and use the trailer mbox1". I think it would make more sense for "-s" to use a trailer called "signoff" if it is configured (and if not, have a baked-in "signoff" trailer config that behaves like "-s" does now). So "-s" (and "--signoff") become "sign off in the way I usually do for my project", not just "add a signed-off-by line". If you want to something more fancy, you have to use "--trailer=...". Just my two cents, as one who has not been closely following this discussion. Apologies if this idea was already presented and shot down. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Wed, Sep 24, 2014 at 12:00:40PM +0200, Christian Couder wrote: > On Tue, Sep 23, 2014 at 10:07 AM, Michael S. Tsirkin wrote: > > On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote: > >> This is probably not as simple as you would like but it works with > >> something like: > >> > >> $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin > >> " --trailer "Reviewed-by: Michael S. Tsirkin > >> " --trailer "Tested-by: Michael S. Tsirkin > >> " 0001-foo.patch >to_apply/0001-foo.patch > >> > >> and then: > >> > >> $ git am to_apply/*.patch > >> > >> Also by using something like: > >> > >> $ git config trailer.a.key Acked-by > >> $ git config trailer.r.key Reviewed-by > >> $ git config trailer.t.key Tested-by > > > > I would like multiple keys to match a specific > > letter, e.g. as a maintainer I need > > both reviewed by and signed off by when I > > apply a patch, I like applying them with > > a single "-s m". > > That's different from what you implemented in your patch. > And franckly I think that for this kind of specific use cases, you > could create your own aliases, either Git aliases or just shell > aliases. > > For example if we implement default values and make git am call git > interpret-trailers, a shell alias could simply be: > > alias gamsm='git am --trailer r --trailer s' > > I use "git log --oneline --decorate --graph" very often, so I made my > own alias for it, and I suppose a lot of other people have done so. > > The number of people who will use trailers will probably be much > smaller than the number of people using git log, so if we don't make > shortcuts for "git log --oneline --decorate --graph", I see no ground > to ask for a specific shortcut that adds both a reviewed by and a > signed off by. I've been thinking: how about a generic ability to add option shortcuts for commands in .config? For example: [am "-z"] command = "--trailer foobar" would replace any -z in git am command line with --trailer foobar. Does this sound useful? > >> the first command could be simplified to: > >> > >> $ git interpret-trailers --trailer "a: Michael S. Tsirkin > >> " --trailer "r: Michael S. Tsirkin " > >> --trailer "t: Michael S. Tsirkin " 0001-foo.patch > >> >to_apply/0001-foo.patch > >> > >> And if you use an env variable: > >> > >> $ ME="Michael S. Tsirkin " > >> $ git interpret-trailers --trailer "a: $ME" --trailer "r: $ME" > >> --trailer "t: $ME" 0001-foo.patch >to_apply/0001-foo.patch > >> > >> Maybe later we will integrate git interpret-trailers with git commit, > >> git am and other commands, so that you can do directly: > >> > >> git am --trailer "a: $ME" --trailer "r: $ME" --trailer "t: $ME" > >> 0001-foo.patch > >> > >> Maybe we wil also assign a one letter shortcut to --trailer, for > >> example "z", so that could be: > >> > >> git am -z "a: $ME" -z "r: $ME" -z "t: $ME" 0001-foo.patch > > > > -s could apply here, right? > > I don't know what we will do with -s. Maybe if we use -z, we don't need -s. > > > It doesn't have a parameter at the moment. > > We will have to discuss that kind of thing when we make it possible > for git commit, git am and maybe other commands to accept trailers > arguments and pass them to git interpret-trailers. > > In his email Junio seems to say that we don't need a shortcut like -z, > we could only have --trailer. > And I think that it is indeed sound to at least wait a little before > using up one shortcut like -z in many commands. > > >> We could also allow many separators in the same -z argument as long as > >> they are separated by say "~", > > > > I think -z a -z r -z t is enough. > > Great! I think you will likely have at least "--trailer a --trailer r > --trailer t", but I don't think it is too bad as you can use aliases > to make it shorter to type. > > >> so you could have: > >> > >> git am -z "a: $ME~r: $ME~t: $ME" 0001-foo.patch > >> > >> And then we could also allow people to define default values for > >> trailers with something like: > >> > >> $ git config trailer.a.defaultvalue "Michael S. Tsirkin " > >> $ git config trailer.r.defaultvalue "Michael S. Tsirkin " > >> $ git config trailer.t.defaultvalue "Michael S. Tsirkin " > > > > I'm kind of confused by the key/value concept. > > A "defaultvalue" would be the value used when no value is passed. > The "key" is just what we will use in the first part of the trailer > (the part before the separator). > > For example with the above "defaultvalue" and "key", "--trailer a: > Junio " would add: > > Acked-by: Junio > > while "--trailer a" would add: > > Acked-by: Michael S. Tsirkin > > > Can't I define the whole 'Acked-by: Michael S. Tsirkin ' > > string as the key? > > The whole 'Acked-by: Michael S. Tsirkin ' is a full > trailer, not a "key". > > And it is not possible right now to define a full trailer. Maybe we > could find a way to make it possible, but a default value and a way to > have a small nickname for the token (like "a" for "A
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 10:15:47AM -0700, Junio C Hamano wrote: > Christian Couder writes: > > > On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin wrote: > >> ... > >> As a reminder, this old patchset (that I replied to) enhanced git am -s > >> with an option to add different signatures depending on > >> the option passed to the -s flag. > >> E.g. I have > >> [am "a"] > >> signoff = "Acked-by: Michael S. Tsirkin " > >> > >> [am "r"] > >> signoff = "Reviewed-by: Michael S. Tsirkin " > >> > >> [am "t"] > >> signoff = "Tested-by: Michael S. Tsirkin " > >> > >> and now: > >> git am -s art > >> adds all 3 signatures when applying the patch. > > > > This is probably not as simple as you would like but it works with > > something like: > > > > $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin > > " --trailer "Reviewed-by: Michael S. Tsirkin > > " --trailer "Tested-by: Michael S. Tsirkin > > " 0001-foo.patch >to_apply/0001-foo.patch > > > > and then: > > > > $ git am to_apply/*.patch > > If I understand it correctly, Michael is envisioning to implement > his "git am -s art" (I would recommend against reusing -s for this, > though. "git am --trailer art" is fine) and doing so by using > interpret-trailers as an internal implementation detail, so I would > say the above is a perfectly fine way to do so. An equivalent of > that command line is synthesized and run internally in his version > of "git am" when his "git am" sees "--trailer art" option using > those am.{"a","r","t"}.trailer configuration variables. Hmm I wonder why do you dislike reusing -s with a parameter for this. To me, this looks like a superset of the default -s functionality: -s adds the default signature, -s "x" adds signature "x" ... Users don't really care that one is implemented as a trailer and another isn't. In fact, default -s can be implemented as a trailer too, right? Could you clarify please? -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
From: Junio C Hamano > On Thu, Sep 25, 2014 at 3:04 AM, Christian Couder > wrote: >>> To an existing message ends with Michael's Signed-off-by:, if his >>> "git am --trailer arts" is called to add these three and then a >>> Signed-off-by: from him, should it add an extra S-o-b (because his >>> existing S-o-b will no longer be the last one after adding Acked and >>> others), or should it refrain from doing so? Can you express both >>> preferences? >> >> The default for "trailer.where" is "end", and for "trailer.ifexists" >> it is "addIfDifferentNeighbor". >> That means that by default it will add the four new trailers at the end. >> >> If either "trailer.ifexists" or "trailer.S-o-b.ifexists" is set to >> "addIfDifferent", then only the first 3 new trailers will be added at >> the end. So yes you can express both preferences. > > Suppose that the original ends with Sob, and Michael's "am" invokes > interpret-trailers with Acked/Reviewed/Tested/Sob in this order. The first > three are set to addifDifferent and Sob is set to addIfDifferentNeighbor, > which would be the traditional and sensible default. > > Now, how is addIfDifferentNeighbor interpreted? Adding the new Sob > with this single request to add these four is done on a message that > has the same Sob at the end (i.e. Neighbor). Does _you_ adding of > Acked/Reviewed/Tested invalidate the Neighbor-ness and you add the > Sob anew? The trailers are processed in the order they appear on the command line. And when a trailer is processed, the input message it "sees" is the result from the processing of all the previous trailers on the command line. It is not any more the original input message. So after Acked/Reviewed/Tested have been added, when the S-o-b at the end of the command line is processed, it "sees" an input message that has the Tested trailer at the end. That's why with addIfDifferentNeighbor the S-o-b will still be added at the end. > If that is what happens, it is not a workable workaround to set Sob to > addIfDifferent only for this invocation. Setting S-o-b to addIfDifferent for this invocation would not add the S-o-b at the end, because another S-o-b still exists in the input message "seen" when the last S-o-b is processed. > Alternatively, if Neighbor-ness is evaluated first before you add A/R/T > in response to this request, then you'd refrain from adding a duplicate > Sob. It wasn't quite clear from your description what your design was, > and your explanation above is not still clear, at least to me. I hope it is clearer now. Maybe I should add something in the doc to better explain how it works. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Thu, Sep 25, 2014 at 3:04 AM, Christian Couder wrote: >> To an existing message ends with Michael's Signed-off-by:, if his >> "git am --trailer arts" is called to add these three and then a >> Signed-off-by: from him, should it add an extra S-o-b (because his >> existing S-o-b will no longer be the last one after adding Acked and >> others), or should it refrain from doing so? Can you express both >> preferences? > > The default for "trailer.where" is "end", and for "trailer.ifexists" > it is "addIfDifferentNeighbor". > That means that by default it will add the four new trailers at the end. > > If either "trailer.ifexists" or "trailer.S-o-b.ifexists" is set to > "addIfDifferent", then only the first 3 new trailers will be added at > the end. So yes you can express both preferences. Suppose that the original ends with Sob, and Michael's "am" invokes interpret-trailers with Acked/Reviewed/Tested/Sob in this order. The first three are set to addifDifferent and Sob is set to addIfDifferentNeighbor, which would be the traditional and sensible default. Now, how is addIfDifferentNeighbor interpreted? Adding the new Sob with this single request to add these four is done on a message that has the same Sob at the end (i.e. Neighbor). Does _you_ adding of Acked/Reviewed/Tested invalidate the Neighbor-ness and you add the Sob anew? If that is what happens, it is not a workable workaround to set Sob to addIfDifferent only for this invocation. Alternatively, if Neighbor-ness is evaluated first before you add A/R/T in response to this request, then you'd refrain from adding a duplicate Sob. It wasn't quite clear from your description what your design was, and your explanation above is not still clear, at least to me. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 7:15 PM, Junio C Hamano wrote: > Christian Couder writes: > >> >> This is probably not as simple as you would like but it works with >> something like: >> >> $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin >> " --trailer "Reviewed-by: Michael S. Tsirkin >> " --trailer "Tested-by: Michael S. Tsirkin >> " 0001-foo.patch >to_apply/0001-foo.patch >> >> and then: >> >> $ git am to_apply/*.patch > > If I understand it correctly, Michael is envisioning to implement > his "git am -s art" (I would recommend against reusing -s for this, > though. "git am --trailer art" is fine) and doing so by using > interpret-trailers as an internal implementation detail, so I would > say the above is a perfectly fine way to do so. An equivalent of > that command line is synthesized and run internally in his version > of "git am" when his "git am" sees "--trailer art" option using > those am.{"a","r","t"}.trailer configuration variables. Yeah, that's the idea, except that I think "--trailer art" should mean a trailer like: art: (if there is no trailer.art.key config variable defined). Having am.{"a","r","t"}.trailer configuration variables to define full trailers seems too specific and quite confusing regarding how git interpret-trailers work without those variables. >> Also by using something like: >> >> $ git config trailer.a.key Acked-by >> $ git config trailer.r.key Reviewed-by >> $ git config trailer.t.key Tested-by >> >> the first command could be simplified to: > > So I think this mechanism buys Michael's use case very little, if > any. It might be useful in other contexts, though. > > What would be more interesting is if the primitives you have, > e.g. "replace", "append", etc. are sufficient to express his use > case and similar ones. For example, when working on multiple > trailers (e.g. "am --trailer art" would muck with three kinds), how > should "do this if exists at the end and do that otherwise" work? The way the "trailer..ifexists" and "trailer..where" work is quite orthogonal to the way we decide what the content of the trailer is. If we make "--trailer art" mean "--trailer Acked-by: Michael --trailer Reviewed-by: Michael --trailer Tested-by: Michael", then it should work as if we had passed the latter to the command line. > To an existing message ends with Michael's Signed-off-by:, if his > "git am --trailer arts" is called to add these three and then a > Signed-off-by: from him, should it add an extra S-o-b (because his > existing S-o-b will no longer be the last one after adding Acked and > others), or should it refrain from doing so? Can you express both > preferences? The default for "trailer.where" is "end", and for "trailer.ifexists" it is "addIfDifferentNeighbor". That means that by default it will add the four new trailers at the end. If either "trailer.ifexists" or "trailer.S-o-b.ifexists" is set to "addIfDifferent", then only the first 3 new trailers will be added at the end. So yes you can express both preferences. > Another thing that got me wondered this morning while I was thinking > about this topic was if "replace" is flexible enough. We may want > to have "if an entry exists (not necessarily at the end), remove it > and then append a new one with this value at the end" to implement > "Last-tested-by: me@my.domain", for example. That's what "replace" does already. That's why I changed the previous name for this option from "overwrite" to "replace". You have an "overwrite behavior" with where = after and ifexist = replace, and you have a "remove old one and append new one behavior" with where = end and ifexist = replace. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Junio C Hamano writes: > What would be more interesting is if the primitives you have, > e.g. "replace", "append", etc. are sufficient to express his use > case and similar ones. For example, when working on multiple > trailers (e.g. "am --trailer art" would muck with three kinds), how > should "do this if exists at the end and do that otherwise" work? > To an existing message ends with Michael's Signed-off-by:, if his > "git am --trailer arts" is called to add these three and then a > Signed-off-by: from him, should it add an extra S-o-b (because his > existing S-o-b will no longer be the last one after adding Acked and > others), or should it refrain from doing so? Can you express both > preferences? By the way, the answer to this can be "no, but it does not matter.", of course. If you can only express the latter (i.e. the addition of multiple trailers is done as an atomic event, what was the last before addition of them will be treated during the whole time of addition of all of them), that may be perfectly fine because the former (i.e. the addition is done one by one) can easily be emulated by calling the program multiple times, feeding the trailers one by one. > Another thing that got me wondered this morning while I was thinking > about this topic was if "replace" is flexible enough. We may want > to have "if an entry exists (not necessarily at the end), remove it > and then append a new one with this value at the end" to implement > "Last-tested-by: me@my.domain", for example. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 10:07 AM, Michael S. Tsirkin wrote: > On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote: >> This is probably not as simple as you would like but it works with >> something like: >> >> $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin >> " --trailer "Reviewed-by: Michael S. Tsirkin >> " --trailer "Tested-by: Michael S. Tsirkin >> " 0001-foo.patch >to_apply/0001-foo.patch >> >> and then: >> >> $ git am to_apply/*.patch >> >> Also by using something like: >> >> $ git config trailer.a.key Acked-by >> $ git config trailer.r.key Reviewed-by >> $ git config trailer.t.key Tested-by > > I would like multiple keys to match a specific > letter, e.g. as a maintainer I need > both reviewed by and signed off by when I > apply a patch, I like applying them with > a single "-s m". That's different from what you implemented in your patch. And franckly I think that for this kind of specific use cases, you could create your own aliases, either Git aliases or just shell aliases. For example if we implement default values and make git am call git interpret-trailers, a shell alias could simply be: alias gamsm='git am --trailer r --trailer s' I use "git log --oneline --decorate --graph" very often, so I made my own alias for it, and I suppose a lot of other people have done so. The number of people who will use trailers will probably be much smaller than the number of people using git log, so if we don't make shortcuts for "git log --oneline --decorate --graph", I see no ground to ask for a specific shortcut that adds both a reviewed by and a signed off by. >> the first command could be simplified to: >> >> $ git interpret-trailers --trailer "a: Michael S. Tsirkin >> " --trailer "r: Michael S. Tsirkin " >> --trailer "t: Michael S. Tsirkin " 0001-foo.patch >> >to_apply/0001-foo.patch >> >> And if you use an env variable: >> >> $ ME="Michael S. Tsirkin " >> $ git interpret-trailers --trailer "a: $ME" --trailer "r: $ME" >> --trailer "t: $ME" 0001-foo.patch >to_apply/0001-foo.patch >> >> Maybe later we will integrate git interpret-trailers with git commit, >> git am and other commands, so that you can do directly: >> >> git am --trailer "a: $ME" --trailer "r: $ME" --trailer "t: $ME" >> 0001-foo.patch >> >> Maybe we wil also assign a one letter shortcut to --trailer, for >> example "z", so that could be: >> >> git am -z "a: $ME" -z "r: $ME" -z "t: $ME" 0001-foo.patch > > -s could apply here, right? I don't know what we will do with -s. Maybe if we use -z, we don't need -s. > It doesn't have a parameter at the moment. We will have to discuss that kind of thing when we make it possible for git commit, git am and maybe other commands to accept trailers arguments and pass them to git interpret-trailers. In his email Junio seems to say that we don't need a shortcut like -z, we could only have --trailer. And I think that it is indeed sound to at least wait a little before using up one shortcut like -z in many commands. >> We could also allow many separators in the same -z argument as long as >> they are separated by say "~", > > I think -z a -z r -z t is enough. Great! I think you will likely have at least "--trailer a --trailer r --trailer t", but I don't think it is too bad as you can use aliases to make it shorter to type. >> so you could have: >> >> git am -z "a: $ME~r: $ME~t: $ME" 0001-foo.patch >> >> And then we could also allow people to define default values for >> trailers with something like: >> >> $ git config trailer.a.defaultvalue "Michael S. Tsirkin " >> $ git config trailer.r.defaultvalue "Michael S. Tsirkin " >> $ git config trailer.t.defaultvalue "Michael S. Tsirkin " > > I'm kind of confused by the key/value concept. A "defaultvalue" would be the value used when no value is passed. The "key" is just what we will use in the first part of the trailer (the part before the separator). For example with the above "defaultvalue" and "key", "--trailer a: Junio " would add: Acked-by: Junio while "--trailer a" would add: Acked-by: Michael S. Tsirkin > Can't I define the whole 'Acked-by: Michael S. Tsirkin ' > string as the key? The whole 'Acked-by: Michael S. Tsirkin ' is a full trailer, not a "key". And it is not possible right now to define a full trailer. Maybe we could find a way to make it possible, but a default value and a way to have a small nickname for the token (like "a" for "Acked-by") should get people quite far. And then for very specific use cases, it may be better to use aliases anyway. >> So that in the end you could have: >> >> git am -z a~r~t 0001-foo.patch >> >> which is very close to "git am -s art". > > If I figure out the defaultvalue thing, I might > find the time to work on git am integration. That would be great! Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Christian Couder writes: > On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin wrote: >> ... >> As a reminder, this old patchset (that I replied to) enhanced git am -s >> with an option to add different signatures depending on >> the option passed to the -s flag. >> E.g. I have >> [am "a"] >> signoff = "Acked-by: Michael S. Tsirkin " >> >> [am "r"] >> signoff = "Reviewed-by: Michael S. Tsirkin " >> >> [am "t"] >> signoff = "Tested-by: Michael S. Tsirkin " >> >> and now: >> git am -s art >> adds all 3 signatures when applying the patch. > > This is probably not as simple as you would like but it works with > something like: > > $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin > " --trailer "Reviewed-by: Michael S. Tsirkin > " --trailer "Tested-by: Michael S. Tsirkin > " 0001-foo.patch >to_apply/0001-foo.patch > > and then: > > $ git am to_apply/*.patch If I understand it correctly, Michael is envisioning to implement his "git am -s art" (I would recommend against reusing -s for this, though. "git am --trailer art" is fine) and doing so by using interpret-trailers as an internal implementation detail, so I would say the above is a perfectly fine way to do so. An equivalent of that command line is synthesized and run internally in his version of "git am" when his "git am" sees "--trailer art" option using those am.{"a","r","t"}.trailer configuration variables. > Also by using something like: > > $ git config trailer.a.key Acked-by > $ git config trailer.r.key Reviewed-by > $ git config trailer.t.key Tested-by > > the first command could be simplified to: So I think this mechanism buys Michael's use case very little, if any. It might be useful in other contexts, though. What would be more interesting is if the primitives you have, e.g. "replace", "append", etc. are sufficient to express his use case and similar ones. For example, when working on multiple trailers (e.g. "am --trailer art" would muck with three kinds), how should "do this if exists at the end and do that otherwise" work? To an existing message ends with Michael's Signed-off-by:, if his "git am --trailer arts" is called to add these three and then a Signed-off-by: from him, should it add an extra S-o-b (because his existing S-o-b will no longer be the last one after adding Acked and others), or should it refrain from doing so? Can you express both preferences? Another thing that got me wondered this morning while I was thinking about this topic was if "replace" is flexible enough. We may want to have "if an entry exists (not necessarily at the end), remove it and then append a new one with this value at the end" to implement "Last-tested-by: me@my.domain", for example. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote: > Hi Michael, > > On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin wrote: > > > > Hi Junio, Christian, > > it's been a while. > > I see that the work on trailers is going on. > > I tried going over the documentation but I could not figure > > out how would one implement multiple signatures using the > > trailers mechanism. > > > > As a reminder, this old patchset (that I replied to) enhanced git am -s > > with an option to add different signatures depending on > > the option passed to the -s flag. > > E.g. I have > > [am "a"] > > signoff = "Acked-by: Michael S. Tsirkin " > > > > [am "r"] > > signoff = "Reviewed-by: Michael S. Tsirkin " > > > > [am "t"] > > signoff = "Tested-by: Michael S. Tsirkin " > > > > and now: > > git am -s art > > adds all 3 signatures when applying the patch. > > This is probably not as simple as you would like but it works with > something like: > > $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin > " --trailer "Reviewed-by: Michael S. Tsirkin > " --trailer "Tested-by: Michael S. Tsirkin > " 0001-foo.patch >to_apply/0001-foo.patch > > and then: > > $ git am to_apply/*.patch > > Also by using something like: > > $ git config trailer.a.key Acked-by > $ git config trailer.r.key Reviewed-by > $ git config trailer.t.key Tested-by I would like multiple keys to match a specific letter, e.g. as a maintainer I need both reviewed by and signed off by when I apply a patch, I like applying them with a single "-s m". > the first command could be simplified to: > > $ git interpret-trailers --trailer "a: Michael S. Tsirkin > " --trailer "r: Michael S. Tsirkin " > --trailer "t: Michael S. Tsirkin " 0001-foo.patch > >to_apply/0001-foo.patch > > And if you use an env variable: > > $ ME="Michael S. Tsirkin " > $ git interpret-trailers --trailer "a: $ME" --trailer "r: $ME" > --trailer "t: $ME" 0001-foo.patch >to_apply/0001-foo.patch > > Maybe later we will integrate git interpret-trailers with git commit, > git am and other commands, so that you can do directly: > > git am --trailer "a: $ME" --trailer "r: $ME" --trailer "t: $ME" > 0001-foo.patch > > Maybe we wil also assign a one letter shortcut to --trailer, for > example "z", so that could be: > > git am -z "a: $ME" -z "r: $ME" -z "t: $ME" 0001-foo.patch -s could apply here, right? It doesn't have a parameter at the moment. > We could also allow many separators in the same -z argument as long as > they are separated by say "~", I think -z a -z r -z t is enough. > so you could have: > > git am -z "a: $ME~r: $ME~t: $ME" 0001-foo.patch > > And then we could also allow people to define default values for > trailers with something like: > > $ git config trailer.a.defaultvalue "Michael S. Tsirkin " > $ git config trailer.r.defaultvalue "Michael S. Tsirkin " > $ git config trailer.t.defaultvalue "Michael S. Tsirkin " I'm kind of confused by the key/value concept. Can't I define the whole 'Acked-by: Michael S. Tsirkin ' string as the key? > So that in the end you could have: > > git am -z a~r~t 0001-foo.patch > > which is very close to "git am -s art". > > Best, > Christian. If I figure out the defaultvalue thing, I might find the time to work on git am integration. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Hi Michael, On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin wrote: > > Hi Junio, Christian, > it's been a while. > I see that the work on trailers is going on. > I tried going over the documentation but I could not figure > out how would one implement multiple signatures using the > trailers mechanism. > > As a reminder, this old patchset (that I replied to) enhanced git am -s > with an option to add different signatures depending on > the option passed to the -s flag. > E.g. I have > [am "a"] > signoff = "Acked-by: Michael S. Tsirkin " > > [am "r"] > signoff = "Reviewed-by: Michael S. Tsirkin " > > [am "t"] > signoff = "Tested-by: Michael S. Tsirkin " > > and now: > git am -s art > adds all 3 signatures when applying the patch. This is probably not as simple as you would like but it works with something like: $ git interpret-trailers --trailer "Acked-by: Michael S. Tsirkin " --trailer "Reviewed-by: Michael S. Tsirkin " --trailer "Tested-by: Michael S. Tsirkin " 0001-foo.patch >to_apply/0001-foo.patch and then: $ git am to_apply/*.patch Also by using something like: $ git config trailer.a.key Acked-by $ git config trailer.r.key Reviewed-by $ git config trailer.t.key Tested-by the first command could be simplified to: $ git interpret-trailers --trailer "a: Michael S. Tsirkin " --trailer "r: Michael S. Tsirkin " --trailer "t: Michael S. Tsirkin " 0001-foo.patch >to_apply/0001-foo.patch And if you use an env variable: $ ME="Michael S. Tsirkin " $ git interpret-trailers --trailer "a: $ME" --trailer "r: $ME" --trailer "t: $ME" 0001-foo.patch >to_apply/0001-foo.patch Maybe later we will integrate git interpret-trailers with git commit, git am and other commands, so that you can do directly: git am --trailer "a: $ME" --trailer "r: $ME" --trailer "t: $ME" 0001-foo.patch Maybe we wil also assign a one letter shortcut to --trailer, for example "z", so that could be: git am -z "a: $ME" -z "r: $ME" -z "t: $ME" 0001-foo.patch We could also allow many separators in the same -z argument as long as they are separated by say "~", so you could have: git am -z "a: $ME~r: $ME~t: $ME" 0001-foo.patch And then we could also allow people to define default values for trailers with something like: $ git config trailer.a.defaultvalue "Michael S. Tsirkin " $ git config trailer.r.defaultvalue "Michael S. Tsirkin " $ git config trailer.t.defaultvalue "Michael S. Tsirkin " So that in the end you could have: git am -z a~r~t 0001-foo.patch which is very close to "git am -s art". Best, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
"Michael S. Tsirkin" writes: > Hi Junio, Christian, > it's been a while. > I see that the work on trailers is going on. > I tried going over the documentation but I could not figure > out how would one implement multiple signatures using the > trailers mechanism. Good. Christian has been rerolling the series numerous times but he has been working without getting much input from people who would envision using the mechanism themselves (and my comments on the series do not count because I am not as one of them). Please bombard us with questions and usability improvement suggestions ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Wed, Jun 18, 2014 at 10:51:04AM -0700, Junio C Hamano wrote: > Junio C Hamano writes: > > > On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin wrote: > >> > >> OK, after looking into this for a while, I realize > >> this is a special property of the Signed-off-by footer. > >> For now I think it's reasonable to just avoid de-duplicating > >> other footers if any. Agree? > > > > Not really. I'd rather see "git am" hardcode as little such policy as > > possible. > > We do need to support S-o-b footer and the policy we defined for it long > > time > > ago, if only for backward compatiblity, but for any other footers, > > policy decision > > such as "dedup by default" isn't something "am" should know about. > > By the way, "append without looking for dups" is a policy decision > that is as bad to have as "append with dedup". > > I'd rather not to see "am.signoff", or any name that implies what > the "-s" option to the command is about for that matter, to be used > in futzing with the trailers other than S-o-b in any way. As I > understand it, our longer term goal is to defer that task, including > the user-programmable policy decisions, to something like the > 'trailer' Christian started. > > I suspect that it may add unnecessary work later if we overloaded > "signoff" with a similar feature with the change under discussion. > I would feel safer to see it outlined how we envision to transition > to a more generic 'trailer' solution later if we were to enhance > "am" with "am.signoff" now. > > Thanks. Hi Junio, Christian, it's been a while. I see that the work on trailers is going on. I tried going over the documentation but I could not figure out how would one implement multiple signatures using the trailers mechanism. As a reminder, this old patchset (that I replied to) enhanced git am -s with an option to add different signatures depending on the option passed to the -s flag. E.g. I have [am "a"] signoff = "Acked-by: Michael S. Tsirkin " [am "r"] signoff = "Reviewed-by: Michael S. Tsirkin " [am "t"] signoff = "Tested-by: Michael S. Tsirkin " and now: git am -s art adds all 3 signatures when applying the patch. Any help will be appreciated. -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Wed, Jun 18, 2014 at 10:51:04AM -0700, Junio C Hamano wrote: > Junio C Hamano writes: > > > On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin wrote: > >> > >> OK, after looking into this for a while, I realize > >> this is a special property of the Signed-off-by footer. > >> For now I think it's reasonable to just avoid de-duplicating > >> other footers if any. Agree? > > > > Not really. I'd rather see "git am" hardcode as little such policy as > > possible. > > We do need to support S-o-b footer and the policy we defined for it long > > time > > ago, if only for backward compatiblity, but for any other footers, > > policy decision > > such as "dedup by default" isn't something "am" should know about. > > By the way, "append without looking for dups" is a policy decision > that is as bad to have as "append with dedup". > > I'd rather not to see "am.signoff", or any name that implies what > the "-s" option to the command is about for that matter, to be used > in futzing with the trailers other than S-o-b in any way. As I > understand it, our longer term goal is to defer that task, including > the user-programmable policy decisions, to something like the > 'trailer' Christian started. > > I suspect that it may add unnecessary work later if we overloaded > "signoff" with a similar feature with the change under discussion. > I would feel safer to see it outlined how we envision to transition > to a more generic 'trailer' solution later if we were to enhance > "am" with "am.signoff" now. > > Thanks. I'll need to look at trailers, if indeed they are a superset of this functionality, I will transition to trailers when they land on master. In this case seems easier for me to keep this out tree patch for now, Good thing I didn't spend time writing docs and tests :) -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Junio C Hamano writes: > On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin wrote: >> >> OK, after looking into this for a while, I realize >> this is a special property of the Signed-off-by footer. >> For now I think it's reasonable to just avoid de-duplicating >> other footers if any. Agree? > > Not really. I'd rather see "git am" hardcode as little such policy as > possible. > We do need to support S-o-b footer and the policy we defined for it long time > ago, if only for backward compatiblity, but for any other footers, > policy decision > such as "dedup by default" isn't something "am" should know about. By the way, "append without looking for dups" is a policy decision that is as bad to have as "append with dedup". I'd rather not to see "am.signoff", or any name that implies what the "-s" option to the command is about for that matter, to be used in futzing with the trailers other than S-o-b in any way. As I understand it, our longer term goal is to defer that task, including the user-programmable policy decisions, to something like the 'trailer' Christian started. I suspect that it may add unnecessary work later if we overloaded "signoff" with a similar feature with the change under discussion. I would feel safer to see it outlined how we envision to transition to a more generic 'trailer' solution later if we were to enhance "am" with "am.signoff" now. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Jun 17, 2014 at 11:49:11PM -0700, Junio C Hamano wrote: > On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin wrote: > > > > OK, after looking into this for a while, I realize > > this is a special property of the Signed-off-by footer. > > For now I think it's reasonable to just avoid de-duplicating > > other footers if any. Agree? > > Not really. I'd rather see "git am" hardcode as little such policy as > possible. > We do need to support S-o-b footer and the policy we defined for it long time > ago, if only for backward compatiblity, but for any other footers, > policy decision > such as "dedup by default" isn't something "am" should know about. OK happily that's exactly what v2 that I just posted does. Default S-o-b footer gets the existing policy. Any other footers are added on top without any tricky deduplication. -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Jun 17, 2014 at 8:09 PM, Michael S. Tsirkin wrote: > > OK, after looking into this for a while, I realize > this is a special property of the Signed-off-by footer. > For now I think it's reasonable to just avoid de-duplicating > other footers if any. Agree? Not really. I'd rather see "git am" hardcode as little such policy as possible. We do need to support S-o-b footer and the policy we defined for it long time ago, if only for backward compatiblity, but for any other footers, policy decision such as "dedup by default" isn't something "am" should know about. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Mon, Jun 16, 2014 at 11:06:20AM -0700, Junio C Hamano wrote: > "Michael S. Tsirkin" writes: > > > Now A wants to sign this patch. > > > > I think there are two reasonable ways to behave: > > 1. What you describe above: > > A > > B > > A > > That is the only sensible thing to do for Signed-off-by footers. OK, after looking into this for a while, I realize this is a special property of the Signed-off-by footer. For now I think it's reasonable to just avoid de-duplicating other footers if any. Agree? To have it apply to other footers, will have to implement ^[A-Z]*-by: logic that I have implemented for sendemail, in am as well. I'll get back to that, but I think it's separate from this feature. -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
"Michael S. Tsirkin" writes: > Now A wants to sign this patch. > > I think there are two reasonable ways to behave: > 1. What you describe above: > A > B > A That is the only sensible thing to do for Signed-off-by footers. > 2. For things like Tested-by: tags, removing tag from > where it was and adding it at the bottom: > > B > A You can make it arbitrarily more complex. A sends with sign-off, B tweaks and tests *that* version and forwards with sign-off, A further tweaks, which may invalidate the earlier tests, so A asks B to retest and then forward it to Linus when everything checks out. What should the end result look like? SoB: A SoB: B Tested-by: B SoB: A Tested-by: B SoB: B perhaps? > This probably calls for a separate feature: > maybe adding "acks" along with "signoffs"? > acks would be unique, re-adding ack removes it from > the message and adds at the bottom. This kind of complication is mostly up to the policies of the projects that are users of Git, and not something we as the Git project want to set in stone inside "git am" implementation. Perhaps consult what has been discussed on Christian's "trailer" series in the list archive and then think about how to integrate / make use of it inside "am"? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Fri, Jun 13, 2014 at 10:32:09AM -0700, Junio C Hamano wrote: > "Michael S. Tsirkin" writes: > > > On Thu, Jun 12, 2014 at 12:07:03PM -0700, Junio C Hamano wrote: > >> "Michael S. Tsirkin" writes: > >> ... > >> > 1. new parameter am.signoff can be used any number > >> > of times: > >> > > >> > [am] > >> > signoff = "Reviewed-by: Michael S. Tsirkin " > >> > signoff = "Signed-off-by: Michael S. Tsirkin " > >> > > >> > if set all signatures are picked up when git am -s is used. > >> > >> How does this interact with the logic to avoid appending the same > >> Signed-off-by: line as the last one the incoming message already > >> has? > > > > Not handled if you have multiple signatures. > > That will have to be fixed. > > Do we only care about the last line? > > > > Signed-off-by: A > > Signed-off-by: B > > > > do we want to add > > > > Signed-off-by: A > > > > or would it be better to replace with > > Signed-off-by: B > > Signed-off-by: A > > > > ? > > > > Current git am will add A twice, I wonder if this is > > a feature or a bug. > > This is very much deliberate. > > Appending A after existing A and B is meant to record that the patch > originated from A, passed thru B possibly with changes by B, came > back to A who wants to assert that the result is still under DCO. > > The only case we can safely omit appending A's sign-off is when the > last one in the chain is by A. Imagine that you had a patch signed > off by B, which A may have tweaked and forwarded under DCO with A's > sign-off. Such a patch would have sign-off chain B-A. > > Now A makes further changes to the patch and says "the further > change is also something I am authorized to release as open source" > with the "-s" option or some other way. It would not change that A > can contribute under DCO if we did not add an extra A after existing > B-A sign-off chain in that case. OK imagine we have signatures: A B Now A wants to sign this patch. I think there are two reasonable ways to behave: 1. What you describe above: A B A 2. For things like Tested-by: tags, removing tag from where it was and adding it at the bottom: B A This probably calls for a separate feature: maybe adding "acks" along with "signoffs"? acks would be unique, re-adding ack removes it from the message and adds at the bottom. -- MST -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
"Michael S. Tsirkin" writes: > On Thu, Jun 12, 2014 at 12:07:03PM -0700, Junio C Hamano wrote: >> "Michael S. Tsirkin" writes: >> ... >> > 1. new parameter am.signoff can be used any number >> >of times: >> > >> > [am] >> >signoff = "Reviewed-by: Michael S. Tsirkin " >> >signoff = "Signed-off-by: Michael S. Tsirkin " >> > >> >if set all signatures are picked up when git am -s is used. >> >> How does this interact with the logic to avoid appending the same >> Signed-off-by: line as the last one the incoming message already >> has? > > Not handled if you have multiple signatures. > That will have to be fixed. > Do we only care about the last line? > > Signed-off-by: A > Signed-off-by: B > > do we want to add > > Signed-off-by: A > > or would it be better to replace with > Signed-off-by: B > Signed-off-by: A > > ? > > Current git am will add A twice, I wonder if this is > a feature or a bug. This is very much deliberate. Appending A after existing A and B is meant to record that the patch originated from A, passed thru B possibly with changes by B, came back to A who wants to assert that the result is still under DCO. The only case we can safely omit appending A's sign-off is when the last one in the chain is by A. Imagine that you had a patch signed off by B, which A may have tweaked and forwarded under DCO with A's sign-off. Such a patch would have sign-off chain B-A. Now A makes further changes to the patch and says "the further change is also something I am authorized to release as open source" with the "-s" option or some other way. It would not change that A can contribute under DCO if we did not add an extra A after existing B-A sign-off chain in that case. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Thu, Jun 12, 2014 at 09:25:54PM +0200, René Scharfe wrote: > Am 12.06.2014 18:12, schrieb Michael S. Tsirkin: > >@@ -136,7 +136,7 @@ fall_back_3way () { > > eval "$cmd" && > > GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \ > > git write-tree >"$dotest/patch-merge-base+" || > >-cannot_fallback "$(gettext "Repository lacks necessary blobs to fall > >back on 3-way merge.")" > >+cannot_fallback "$(gettext "Repository lsignoffs necessary blobs to > >fall back on 3-way merge.")" > > lsignoffs? Heh good catch, I'll fix this up. Thanks! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Thu, Jun 12, 2014 at 12:07:03PM -0700, Junio C Hamano wrote: > "Michael S. Tsirkin" writes: > > > I'm using different signature tags for git am depending on the patch, > > project and other factors. > > > > Sometimes I add multiple tags as well, e.g. QEMU > > wants both Reviewed-by and Signed-off-by tags. > > > > This patch makes it easy to do so: > > 1. new parameter am.signoff can be used any number > > of times: > > > > [am] > > signoff = "Reviewed-by: Michael S. Tsirkin " > > signoff = "Signed-off-by: Michael S. Tsirkin " > > > > if set all signatures are picked up when git am -s is used. > > How does this interact with the logic to avoid appending the same > Signed-off-by: line as the last one the incoming message already > has? Not handled if you have multiple signatures. That will have to be fixed. Do we only care about the last line? Signed-off-by: A Signed-off-by: B do we want to add Signed-off-by: A or would it be better to replace with Signed-off-by: B Signed-off-by: A ? Current git am will add A twice, I wonder if this is a feature or a bug. > > 2. Any number of alternative signatures > > > > [am "a"] > > signoff = "Acked-by: Michael S. Tsirkin " > > > > if set the signature type can be specified by passing > > a parameter to the -s flag: > > > > git am -sa > > > > No docs or tests, sorry, so not yet ready for master, but I'm using this > > all the time without any issues so maybe ok for pu. > > Early flames/feedback/help welcome. > > How does that "a" in [am "a"] work? If it defines some kind of > scope (i.e. use am.a.* instead of am.* when I specify I am using "a" > set somehow), that might be something interesting, but if it applies > only to sign-off and other things, then I am not sure if I like it, > as that would invite confusions from end users. > > > + signoffs=("${signoffs[@]}" "${s[@]}") ;; > > Is this a shell array? It won't fly in our codebase if that is the > case. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
Am 12.06.2014 18:12, schrieb Michael S. Tsirkin: @@ -136,7 +136,7 @@ fall_back_3way () { eval "$cmd" && GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \ git write-tree >"$dotest/patch-merge-base+" || -cannot_fallback "$(gettext "Repository lacks necessary blobs to fall back on 3-way merge.")" +cannot_fallback "$(gettext "Repository lsignoffs necessary blobs to fall back on 3-way merge.")" lsignoffs? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
"Michael S. Tsirkin" writes: > I'm using different signature tags for git am depending on the patch, > project and other factors. > > Sometimes I add multiple tags as well, e.g. QEMU > wants both Reviewed-by and Signed-off-by tags. > > This patch makes it easy to do so: > 1. new parameter am.signoff can be used any number > of times: > > [am] > signoff = "Reviewed-by: Michael S. Tsirkin " > signoff = "Signed-off-by: Michael S. Tsirkin " > > if set all signatures are picked up when git am -s is used. How does this interact with the logic to avoid appending the same Signed-off-by: line as the last one the incoming message already has? > 2. Any number of alternative signatures > > [am "a"] > signoff = "Acked-by: Michael S. Tsirkin " > > if set the signature type can be specified by passing > a parameter to the -s flag: > > git am -sa > > No docs or tests, sorry, so not yet ready for master, but I'm using this > all the time without any issues so maybe ok for pu. > Early flames/feedback/help welcome. How does that "a" in [am "a"] work? If it defines some kind of scope (i.e. use am.a.* instead of am.* when I specify I am using "a" set somehow), that might be something interesting, but if it applies only to sign-off and other things, then I am not sure if I like it, as that would invite confusions from end users. > + signoffs=("${signoffs[@]}" "${s[@]}") ;; Is this a shell array? It won't fly in our codebase if that is the case. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html