Re: [PATCH] MyFirstContribution: Document --range-diff option when writing v2

[Junio C Hamano <gitster@pobox.com>]

Glen Choo <chooglen@google.com> writes:

> +Let's write v2 as its own topic branch, because this will make some things more
> +convenient later on. Create the `psuh-v2` branch like so:
>  
> +----
> +$ git checkout -b psuh-v2 psuh
> +----

What's missing here is on which branch this new description expects
the user to work on.  From its name, I am assuming that psuh-v2 will
be modified while leaving psuh untouched, but spell your expectation
out.

The following review is based on the assumption that the user is
expected to futz with psuh-v2, leaving psuh intact.  If that is not
the case, it is a strong sign that you caused confusion on one
reader by not spelling out your expectation.

I do not think it is a good suggestion at all to use a new topic
branch, especially a one that forked from the tip of the original
submission, and work on that branch to produce the new round.  It
would be much better to create a topic branch or a lightweight tag
"psuh-v1" that points at the old tip and keep working on the same
branch.  But that is a separate story.

> +When you're ready with the next iteration of your patch, the process is fairly
> +similar to before. Generate your patches again, but with some new flags:
>  
>  ----
> -$ git format-patch -v2 --cover-letter -o psuh/ master..psuh
> +$ git format-patch -v2 --range-diff psuh..psuh-v2 --cover-letter -o psuh/ master..psuh
>  ----

Before the "Generate your patches again", there would have been
"rebase -i" of the original commits that went into "psuh" (v1).

But you do not necessarily have to touch all the commits during
"rebase -i" session.  What happens when the first few commits did
not need to be touched?

Since the --range-diff says psuh..psuh-v2, these early and
unmodified commits are excluded from the range, no?  That would mean
what appears to be commit #1 in the range-diff on the new side would
not be the [PATCH 1/n] of the output, no?

And the command line says to format master..psuh, which is
puzzling.  Shouldn't it format the updated psuh-v2 branch?

> +The `--range-diff psuh..psuh-v2` parameter tells `format-patch` to include a
> +range diff between `psuh` and `psuh-v2`. This helps tell reviewers about the
> +differences between your v1 and v2 patches.

See above.  The range-diff may fail to tell the fact that there are
a few bottommost commits that are the same by omitting them.

Perhaps it would make it easier to manage if we used psuh-v1 as the
anchoring point to represent where the tip of the last round was?

With something like:

	$ git checkout psuh
	$ git branch psuh-v1 ;# optional -- "git tag" is also OK.
	... work work work with "rebase -i" ...
	$ git format-patch -v2 --cover-letter -o psuh/ \
		--range-diff master..psuh-v1 master..
	# ..psuh-v1 can be ..@{yesterday} or whatever reflog reference

we do not have to worry if "rebase -i" left the bottommost commits
untouched or silly things like that.  

> +The `-v2` parameter tells `format-patch` to output "v2" patches. For instance,
> +you may notice that your v2 patches, are all named like
> +`v2-000n-my-commit-subject.patch`. `-v2` will also format your patches by
> +prefixing them with "[PATCH V2]" instead of "[PATCH]", and your range-diff will
> +be prefaced with "Range-diff against v1".
> +
> +Afer you run this command, `format-patch` will output the patches to the `psuh/`
> +directory, alongside the v1 patches. That's fine, but be careful when you are
> +ready to send them.

It is unclear what "That's fine, but" is trying to convey.

I'd replace it with something like:

	You can refer to the old v1 patches while giving the final
	proofreading on the v2 patches, but you now need to say "git
	send-email psuh/v2-*.patch" to send them out ("*.patch" would
	match both v1 and v2 patches).

Thanks.