Re: [PATCH 1/4] rebase -i: add ack action

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

On Mon, Apr 11, 2016 at 01:02:07PM +0200, Johannes Schindelin wrote:
> Hi Michael,
> 
> On Sun, 10 Apr 2016, Michael S. Tsirkin wrote:
> 
> > This implements a new ack! action for git rebase -i
> > It is essentially a middle ground between fixup! and squash!:
> > - commits are squashed silently without editor being started
> > - commit logs are concatenated (with action line being discarded)
> > - because of the above, empty commits aren't discarded,
> >   their log is also included.
> > 
> > I am using it as follows:
> > 	git am -s < mailbox #creates first commit
> > 	hack ...
> > 	get mail with Ack
> > 	git commit --allow-empty -m `cat <<-EOF
> > 	ack! first
> > 
> > 	Acked-by: maintainer
> > 	EOF`
> > 	repeat cycle
> > 	git rebase --autosquash -i origin/master
> > 	before public branch push
> > 
> > The "cat" command above is actually a script that
> > parses the Ack mail to create the empty commit -
> > to be submitted separately.
> 
> This looks awfully complicated, still, and not very generic.
> 
> How about making it easier to use, and much, much more generic, like this?

I can look at using a different syntax but the below does not
support the workflow I described, which is a standard
email based one: get email, handle it.

> 1. introducing an `--add-footer` flag to `git commit` that you could use
> like this:
> 
> 	git commit --amend --add-footer "Acked-by: Bugs Bunny"
> 2. introducing an `--exec-after` flag to `git commit` that would be a new
> sibling of `--fixup` and `--squash` and would work like this:
> 
> 	git commit --exec-after HEAD~5 \
> 		'git commit --amend --add-footer "Acked-by: Bugs Bunny"'

But it wouldn't address my use-case where I get an ack
by email. If I have to dig up the relevant commit(s) by hand
anyway, then what was the point?

> 
> (it should imply `--allow-empty`, of course, and probably even fail if
> anything was staged for commit at that point.) The commit message would
> then look something like
> 
> 	exec-after! Fix broken breakage
> 
> 	git commit --amend --add-footer "Acked-by: Bugs Bunny"

So if I happen to fetch a branch from someone
and rebase it, stuff gets auto-executed on my local system?
That looks scary. 

> This way would obviously benefit a lot more users.

It might benefit others who have the commit handy but it does not look
like it helps email based workflow.

> For example, you could
> easily say (and alias)
> 
> 	git commit --amend --add-footer 'Reviewed-by: Arrested Developer"
> 
> i.e. support all kind of use cases where developers need to slap on
> footers in a quick & easy way.
>
> And the --exec-after option would obviously have *a lot* more use cases
> than just squashing in ACKs.
> 
> Ciao,
> Johannes

So far I only see examples of adding footers. If that's all we can think
up, why code in all this genericity?  All these small scripts scattered
around just make things hard to use, and add security issues.


-- 
MSR