Thread (3 messages) 3 messages, 3 authors, 2025-12-20

Re: [PATCH v2 0/3] doc: replay: improvements like "mention no output on conflicts"

From: Kristoffer Haugsbakk <hidden>
Date: 2025-12-20 19:34:59

On Tue, Dec 16, 2025, at 01:29, Junio C Hamano wrote:
"Kristoffer Haugsbakk" [off-list ref] writes:
quoted
[snip]
Thank you. But I’m not glad that the commit message is not clear. I
would need some guidance on how to write it because it seems clear to
me. Something with my brain state I guess.
They are already in 'next', but let's see if there are pain points.

[snip]
commit 03d7c9c457ba68f28269dcd607b9026ea6c6c9c8
Author: Kristoffer Haugsbakk [off-list ref]
Date:   Sat Dec 13 14:46:57 2025 +0100

    replay: improve --contained and add to doc

    There is no documentation for `--contained`.

    Start by copying the text from `replay_options` in `builtin/
    replay.c`. But some people think that the existing text is a
    bit unclear; what does it mean for a branch to be contained
    in a revision range? Let’s include the implied commits here:
    the branches that point at commits in the range.

    Also use “update” instead of “advance”. “Update” is the verb
    commonly used in this context.

    Helped-by: Phillip Wood [off-list ref]
    Helped-by: Junio C Hamano [off-list ref]
    Signed-off-by: Kristoffer Haugsbakk [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

As to the title, "improve --contained" hinted me there is some code
changes for behaviour, but there isn't, so that part may have been a
bit misleading.  "improve short-help of --contained and add to doc",
perhaps.
Oh right, of course. The original area was `doc` and in that case this
would have been fine. But I didn’t consider the `replay` area. So now it
looks like the `--contained` option logic has been changed.
I think the problem people found in the second paragraph is because
it is so unclear what it is talking about if you read it without
looking at the patch text.  You started from the existing "advance
all branches contained in revision-range", taken from the existing
short-help in replay_options[].  But without seeing that "branches
contained" text, it is natural that readers find it hard to judge
the validity of "But some people think that..." claim themselves.

If I were writing this (but I will not rewind 'next' to do so),
I'd say something like:

    replay: improve the help of the `--contained` option and document it

    "git replay -h" explains "--contained" as

	advance all branches contained in revision-range

    but it may be unclear when exactly a branch is contained in a
    revision range.  Because the command updates a branch that
    points at a commit that gets rewritten to point at the result of
    the rewrite, "update branches that point at commits in the
    range" says what we want to say more clearly and concisely.

    The "--contained" option has no description in "git replay"
    documentation.  Use the improved phrase there, too.

probably.  In any case, it is a good exercise to see if the proposed
log message can be easily understood without looking at the code
change.
Okay, now I get it. It turns out I’m still learning how to write commit
messages with the right amount of context.

And thanks to Phillip for confirming.
[snip]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help