From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Date: Fri, 02 Jan 2026 09:51:31 GMT
Subject: Re: [PATCH v2 1/5] replay: remove dead code and rearrange
Message-ID: <3dfe9f23-cf54-4162-a90d-69ef9788d6b9@app.fastmail.com>
In-Reply-To: <CABPp-BEJV1XG62_hn_OiZ9q9S3jsyTP0VdOEzS4pME2rrkKFrg@mail.gmail.com>

On Tue, Dec 30, 2025, at 23:50, Elijah Newren wrote:
> On Tue, Dec 30, 2025 at 7:03 AM <kristofferhaugsbakk@fastmail.com> wrote:
>>[snip]
>> -static void determine_replay_mode(struct repository *repo,
>> -                                 struct rev_cmdline_info *cmd_info,
>> -                                 const char *onto_name,
>> -                                 char **advance_name,
>> -                                 struct commit **onto,
>> -                                 struct strset **update_refs)
>> +static void populate_for_onto_or_advance_mode(struct repository *repo,
>> +                                             struct rev_cmdline_info *cmd_info,
>> +                                             const char *onto_name,
>> +                                             char **advance_name,
>> +                                             struct commit **onto,
>> +                                             struct strset **update_refs)
>
> Renaming makes sense, but the new name is quite the mouthful, and it
> feels slightly odd because "onto" is both a command line flag and a
> variable -- and the variable value is used regardless of which command
> line flag is used.  Since the variable is used either way, there's a
> risk someone might be confused by this function name.  Maybe just
> setup_replay_mode() ?  Or maybe others have other suggestions?

Yeah, it is a mouthful.

I can use `set_up_replay_mode`.

>>[snip]
>
> Looks fine otherwise.

Thanks for reviewing this round!