From: Christian Couder <christian.couder@gmail.com>
Date: Wed, 29 Oct 2025 16:19:32 GMT
Subject: Re: [PATCH v5 3/3] replay: add replay.refAction config option
Message-ID: <CAP8UFD03fx+wKwJzDG8UZz=+S8=07hG6npNnebTmBxrcXNYqGQ@mail.gmail.com>
In-Reply-To: <20251028214609.10041-4-siddharthasthana31@gmail.com>

On Tue, Oct 28, 2025 at 10:46 PM Siddharth Asthana
<siddharthasthana31@gmail.com> wrote:

> +static enum ref_action_mode parse_ref_action_mode(const char *mode_str, const char *source)

Nit: it's a bit strange that it's called "ref_action_str" everywhere
except here where it's called "mode_str". I'd prefer "ref_action"
everywhere.

(I understand that "mode" is related to parse_ref_action_mode() having
"mode" in its name but it's the case for get_ref_action_mode() too.)

> +test_expect_success 'replay.refAction config option' '
> +       # Store original state
> +       START=$(git rev-parse topic2) &&
> +       test_when_finished "git branch -f topic2 $START" &&
> +       test_when_finished "git config --unset replay.refAction || true" &&

Is there something preventing test_config to be used in this test
while it's used in other tests below?

> +       # Set config to print
> +       git config replay.refAction print &&
> +       git replay --onto main topic1..topic2 >output &&
> +       test_line_count = 1 output &&
> +       test_grep "^update refs/heads/topic2 " output &&
> +
> +       # Reset and test update mode
> +       git branch -f topic2 $START &&
> +       git config replay.refAction update &&
> +       git replay --onto main topic1..topic2 >output &&
> +       test_must_be_empty output &&
> +
> +       # Verify ref was updated
> +       git log --format=%s topic2 >actual &&
> +       test_write_lines E D M L B A >expect &&
> +       test_cmp expect actual
> +'