Re: [PATCH 2/5] Add git-sequencer documentation

Hi,

Johannes Schindelin wrote:

quoted

+-B::
+--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.

Does it abort, or leave a dirty tree?

It aborts.
Perhaps I should make this clear in the docs.

quoted

+--onto=<base>::
+	Checkout given commit or branch before sequencing.
+	If you provide a branch, sequencer will make the provided
+	changes on the branch, i.e. the branch will be changed.

Whoa, does that mean that

	$ git checkout my-private-branch
	$ git sequencer --onto=master

will change _master_?

Exactly.

quoted

+--continue::
+	Restart the sequencing process after having resolved a merge conflict.

What about 'edit'?  Does it restart the sequencing process after editing a 
file or commit message, too?

Yes. Thanks.

quoted

+If you nonetheless noticed that you made a mistake, you can
+overwrite `.git/sequencer/todo` with `.git/sequencer/todo.old` and
+rerun `git sequencer --edit`.

Speaking of "todo": there was an explicit request to change that to 
"git-rebase-todo" for rebase -i, so that syntax highlighting could be 
switched on.

I'd not have a problem with that, though the name "rebase" is not
necessarily correct and the syntax files had to be extended
nevertheless.

quoted

+-v::
+--verbose::
+	Be more verbose.

More?

Hehe, the note that "more" has to be defined more accurately, has been
removed. But it is still true. ;)

quoted

+a branch in a way that can cause problems for anyone who already has
+a copy of the branch in their repository and tries to pull updates from
+you.  You should understand the implications of using 'git-sequencer' on
+a repository that you share.

How about this instead?

	Note that sequencing will rewrite the history of the branch.  

It will not necessarily rewrite history of a branch. In case of the
rebase user command, it does, of course.

	This will cause problems if you published the branch prior to
	rewriting the history, as the former tip is no longer an 
	ancestor of the new tip.

	In other words, if you rewrite an already published branch, users 
	that pull from you _will_ get a bogus merge.

Yes, I can take that.

Well, I wanted only a short note and the user command that really
rewrites branches could have a more detailed warning.

quoted

+'git-sequencer' will usually be called by another git porcelain, like

s/another git procelain/other git programs/

Ok, I use "other git commands", since this wording seems to be the leading
one in other documentation.

quoted

+TODO FILE FORMAT
+----------------
+
+The TODO file contains basically one instruction per line.

s/basically //

Oh, this is from the times where we included some possible extensions,
like a trailing backslash.

quoted

+edit <commit>::
+	Pick a commit and pause the sequencer process to let you
+	make changes.
++
+This is a short form for `pick <commit> and `pause` on separate lines.

It might make sense to explain 'pick' before 'edit', then.

This is kept alphabetically.

When first writing that, I wondered if this makes sense:
a reference-like list should be sorted alphabetically, a tutorial-like
list should have some kind of logical structure.
I do not really care if we prefer this or that one. So I take your
reordering suggestion until somebody complains again :)

quoted

+	--mainline=<n>;;
+		Allow you to pick merge commits by specifying the
+		parent number (beginning from 1) to let sequencer
+		replay the changes relative to the specified parent.

Why is this called "mainline", and not "parent"?

Because git-cherry-pick/git-revert has "--mainline" and I did not want
to rename this.

quoted

[talking about 'squash']

+	--collect-signoffs;;
+		Collect the Signed-off-by: lines of each commit and
+		add them to the squashed commit message.
+		(Not yet implemented.)

I really have to wonder how useful that is.  Or how correct, for that 
matter.

This is just some kind of squashing Signed-off-by: lines.
It was requested by someone (Paolo?) in the RFC git-sequencer.txt thread.

The behavior is planned like this:

Instead of...

	Message of commit 1
	
	Signed-off-by: A
	
	Message of commit 2
	
	Signed-off-by: B
	
	Message of commit 3
	
	Signed-off-by: A
	Signed-off-by: C

...the user gets...

	Message of commit 1
	
	Message of commit 2
	
	Message of commit 3
	
	Signed-off-by: A
	Signed-off-by: B
	Signed-off-by: C

using --collect-signoffs and no commit-message-changing options.

With --message="Foo" this will be:

	Foo
	
	Signed-off-by: A
	Signed-off-by: B
	Signed-off-by: C

For me this sounds like making sense and being useful, but I have not yet
digged so deep into the rules of signing off :)

quoted

+	--include-merges;;
+		Sanity check does not fail if you have merges
+		between HEAD and <mark>.

It may be a commit, too, right?  And why does it make sense to check that 
there are no merges?  I mean, it is just as if I did two cherry-picks, the 
second with -n, and then commit --amend it.  Can make tons of sense...

I think I mean something different. With "merges" I am talking about
commits having more than one parent.
By this check I want to make sure, that the user really knows what he
does when he wants to squash the ones marked with ~ into one commit.

         ~~~~~~~~~~~~~
  ..-A---B---C---D---E---F
            /
  ..-X---Y--

Yet I didn't really care what happens then and perhaps it should be:

 ..--A---S--F
        /
 ..-X-Y-

In the prototype the squashing is done by soft-resetting to the
squash base and then commit. As I said, I did not really care if
my painted result really happens.  In the builtin it will happen :)

quoted

+Here are some examples that shall ease the start with the TODO
+file format.
+Make sure you have understood the `pick` and perhaps the `patch` command.
+Those will not be explained further.

This sentence is insulting.  Strike it.

The "Make sure ..." sentence?  It really is insulting?
Do you think it implies the assumption that the user will not grasp
patch/pick easily?


Thanks for your reply and your other notes. (The ones I didn't comment
are just ACKed.)

Regards,
  Stephan

-- 
Stephan Beyer [off-list ref], PGP 0x6EDDD207FCC5040F