mbox series

[v2,0/3] sequencer: comment out properly in todo list

Message ID cover.1731406513.git.code@khaugsbakk.name (mailing list archive)
Headers show
Series sequencer: comment out properly in todo list | expand

Message

Kristoffer Haugsbakk Nov. 12, 2024, 10:20 a.m. UTC
From: Kristoffer Haugsbakk <code@khaugsbakk.name>

Fix three places where the comment char/string is hardcoded (#) in the
todo list.

This series does not depend on any other topics.
Topic `jc/strbuf-commented-something` was mentioned on the v1
discussion.  But it was kicked out of `seen` last week.  Also it doesn’t
compile when merged into `v2.47.0` or later:

```
strbuf.c: In function ‘strbuf_add_comment_lines’:
strbuf.c:384:24: error: ‘comment_line_str’ undeclared (first use in this function)
  384 |         add_lines(out, comment_line_str, buf, size, 1);
      |                        ^~~~~~~~~~~~~~~~
strbuf.c:384:24: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:2795: strbuf.o] Error 1
```

§ Changes in v2

The first version just had patch 1 but this one fixes two other places.
The two other places where unearthered during the v1 discussion.

Rebased on `master` (b31fb630c0 (Merge https://github.com/j6t/git-gui,
2024-11-11)).

§ CI

Some failures that didn’t look relevant.

• linux-leaks
  • t0301-credential-cache
  • t9211-scalar-clone
• linux-reftable-leaks
  • ditto above

§ CC

• Stolee for the first patch
• Reviewers on the previous round

Kristoffer Haugsbakk (3):
  sequencer: comment checked-out branch properly
  sequencer: comment `--reference` subject line properly
  sequencer: comment commit messages properly

 sequencer.c                     | 26 ++++++++++++++++----------
 t/t3400-rebase.sh               | 16 ++++++++++++++++
 t/t3437-rebase-fixup-options.sh | 15 +++++++++++++++
 t/t3501-revert-cherry-pick.sh   | 12 ++++++++++++
 4 files changed, 59 insertions(+), 10 deletions(-)

Range-diff against v1:
1:  5267b9a9c8c ! 1:  fc3b4438845 sequencer: comment checked-out branch properly
    @@ Commit message
     
         `git rebase --update-ref` does not insert commands for dependent/sub-
         branches which are checked out.[1]  Instead it leaves a comment about
    -    that fact.  The comment char is hard-coded (#).  In turn the comment
    +    that fact.  The comment char is hardcoded (#).  In turn the comment
         line gets interpreted as an invalid command when `core.commentChar`
         is in use.
     
    @@ Commit message
     
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
    +
    + ## Notes (series) ##
    +    v2:
    +    • Message: “hardcoded” (more common according to `git grep`)
    +
      ## sequencer.c ##
     @@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
      		/* If the branch is checked out, then leave a comment instead. */
-:  ----------- > 2:  710c5b1a3f6 sequencer: comment `--reference` subject line properly
-:  ----------- > 3:  86b4b485e0b sequencer: comment commit messages properly

base-commit: b31fb630c0fc6869a33ed717163e8a1210460d94

Comments

Junio C Hamano Nov. 13, 2024, 12:26 a.m. UTC | #1
kristofferhaugsbakk@fastmail.com writes:

> The first version just had patch 1 but this one fixes two other places.
> The two other places where unearthered during the v1 discussion.

OK.  I guess they could be handled in a single patch, but the three
patches address different things to be commented properly, so having
them as three separate patches is good.

> Rebased on `master` (b31fb630c0 (Merge https://github.com/j6t/git-gui,
> 2024-11-11)).

Was there any reason, other than "newer must be better" (which is
not always true)?  I thought there isn't any in-flight topics that
touched the sequencer machinery.

> Some failures that didn’t look relevant.

Judging from https://github.com/git/git/actions/runs/11774751134
(which you rebased the topic on) passing all these CI tests,
if your topic saw CI breakages, there is nothing else we can put the
blame on.

But let's see what happens.  I won't see CI failures on individual
topic, but we will see what the topic, together with everybody else,
in the context of 'seen', produces when I push today's integration
result out.

Thanks.