Re: [PATCH RFC] git-am: support any number of signatures

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jun 12, 2014 at 12:07:03PM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> 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 <mst@redhat.com>"
> > 	signoff = "Signed-off-by: Michael S. Tsirkin <mst@redhat.com>"
> >
> > 	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 <mst@redhat.com>"
> >
> > 	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.