Glen Choo <chooglen@xxxxxxxxxx> 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.
