From: Stephan Beyer <s-beyer@gmx.net>
Date: Mon, 09 Jun 2008 16:29:09 GMT
Subject: Re: squashing patches
Message-ID: <20080609162909.GC8079@leksak.fem-net>
In-Reply-To: <484D47C9.9050509@gnu.org>

Hi,

> My feedback is in the message from the "-s theirs" thread that I CCed  
> you on.

Well, I've noticed and read it, but haven't knowm what I wanted to 
reply. :)

> Basic points:
>
> 1) I would like a "--strategy" option for cherry-pick and for the  
> sequencer's "pick";

If somebody adds a --strategy option to cherry-pick and it gets
into "next", I have no problem to let sequencer pass the option to
cherry-pick ;)

Btw, I like the idea of cherry-pick --strategy=theirs (though I don't
know if I'd ever use it), but this is currently beyond my task ;-)

> 2) What about
>
>   mark :1
>   pick a
>   file b
>   pick c
>   squash --up-to :1

I like the idea.  And the others?

--- a/Documentation/git-sequencer.txt
+++ b/Documentation/git-sequencer.txt
@@ -212,6 +216,12 @@ squash [<options>] <commit>::
 +
 See 'GENERAL OPTIONS' for values of `<option>`.
 
+squash [<options>] --up-to <mark>::
+	Squash all commits up to the given mark into one commit.
+	There must not be any merge commits in between.
++
+See 'GENERAL OPTIONS' for values of `<option>`.
+
 
 tag <tag>::
 	Set tag `<tag>` to the current HEAD,

> or, to specify a commit message
>
>   mark :1
>   file a
>   pick b
>   squash --up-to :1 -C HEAD^

Reusing the commit message may be useful, too, agreed.
I think that this is something that can be put to the 
"GENERAL OPTIONS" section:

--- a/Documentation/git-sequencer.txt
+++ b/Documentation/git-sequencer.txt
@@ -240,6 +250,12 @@ when finished.
 	Override the author name and e-mail address used in the commit.
 	Use `A U Thor <author@example.com>` format.
 
+-C <commit-ish>::
+--reuse-message=<commit-ish>::
+	Reuse message from specified commit.
+	Note, that only the commit message is reused
+	and not the authorship information.
+
 -F <file>::
 --file=<file>::
 	Take the commit message from the given file.
--

Ok?


If you want also the authorship information, there's the --reference
which is atm only available for the "merge" insn.
Currently I also wonder, why only merge has this option ;-)

Should this be a General Option, too?

--- a/Documentation/git-sequencer.txt
+++ b/Documentation/git-sequencer.txt
@@ -182,9 +189,6 @@ or `--standard`), an editor will be invoked.
 +
 See the following list and 'GENERAL OPTIONS' for values of `<option>`:
 
-	--reference=<commit-ish>;;
-		Take author and commit message of <commit-ish>.
-	
 	--standard;;
 		Generates a commit message like 'Merge ... into HEAD'.
 		See also linkgit:git-fmt-merge-msg[1].
@@ -247,6 +263,10 @@ when finished.
 -m <msg>::
 --message=<msg>::
 	Use the given `<msg>` as the commit message.
+	
+--reference=<commit-ish>::
+	Take author and commit message of <commit-ish>.
+	
 
 -s::
 --signoff::
--

> or also
>
>   mark :1
>   file a
>   pick b
>   squash --up-to :1 -C HEAD^ -s
>
> to merge all signoffs.

Hm, this is a bit too specific I think.
Do I interpret your example right, that you want to 
squash "a" and "b", reuse the commit message of the
newly commited patch "a", but also add the signoff of
"b"? (And perhaps, if not set, the own signoff?)

If I put this into sequencer, I'd like to name
that --squash-signoffs (or --merge-signoffs) or something,
because -s/--signoff is a general option atm, that adds
your signoff.

What do the others think? (Sorry for my questions to "the others", 
but I need some input. *grin*)

> This could be done by providing a stand-alone  
> git-squash command; or alternatively, it could be done in the sequencer  

The git-squash approach looks clean to me.

>   (echo 'mark :1'
>    git-rev-list --reverse $1.. | sed 's,^,pick '
>    echo "squash --up-to :1 $*") | git-sequencer

I notice that you indirectly changed the synopsis, intuitively.
This shows me, that we should change it ;-)
According to the current SYNOPSIS you have to do "git-sequencer -"
to read from stdin.

> 3) I would like a totally batch mode-of-operation, which would fail if  
> user intervention was needed

I like the idea.

--- a/Documentation/git-sequencer.txt
+++ b/Documentation/git-sequencer.txt
@@ -8,7 +8,7 @@ git-sequencer - Execute a sequence of git instructions
 SYNOPSIS
 --------
 [verse]
-'git-sequencer' [-v | --verbose] <file> [<branch>]
+'git-sequencer' [--batch] [-v | --verbose] <file> [<branch>]
 'git-sequencer' --continue | --skip | --abort | --edit

@@ -59,6 +59,13 @@ OPTIONS
 <branch>::
 	Working branch; defaults to HEAD.
 
+--batch::
+	Run in batch mode. If unexpected user intervention is needed
+	(e.g. a conflict or the need to run an editor), git-sequencer fails.
++
+Note that the sanity check fails, if you use this option
+and an instruction like `edit` or the `--edit` option.
+
 --continue::
 	Restart the sequencing process after having resolved a merge conflict.
 
--

Ok?

> (the user could choose whether to not edit the editor,
> or whether to use a no-op for GIT_EDITOR).

I don't understand ;-)

> 4) I think the sequencer is an opportunity to improve some commands,  
> e.g. git-cherry-pick should grow more or less the same options as  
> git-sequencer's pick.

Hmmmmm, basically I like this approach.
Let's for example take the "merge --reference=<commit>" option.
It fetches the information from a former commit, and then does
git merge with different authorship information and -m "message bla".
So the basics are there, they just have to be put together.
Do you think it's useful to add something like --reference (or -C,
or however it is called) to git-merge?

Thanks and regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F