Re: [PATCH v4 0/3] replay: make atomic ref updates the default
- From
Siddharth Asthana <siddharthasthana31@gmail.com>
- Date
- Oct 28, 2025, 20:19 UTC
- Message-ID
- <1a4740b8-eb63-4266-b627-d329451b7a4b@gmail.com>
- In-Reply-To
- <xmqq7bwlv4jh.fsf@gitster.g>
On 24/10/25 00:17, Junio C Hamano wrote:
Show 29 quoted lines
> Siddharth Asthana <siddharthasthana31@gmail.com> writes: > >> This is v4 of the git-replay atomic updates series. >> >> Based on feedback from v3, this version improves the naming and >> implementation for clarity and type safety. Thanks to Junio, Christian, >> Elijah, Phillip, Patrick, and Karthik for the detailed reviews. >> >> ## Changes in v4 >> >> **Renamed --update-refs to --ref-action** >> >> Junio pointed out that "--update-refs=print" is semantically awkward. >> Answering "print" to the question "update refs?" doesn't make sense. >> The actual question is "what action should we take on the refs?" >> >> Changed to --ref-action=(update|print) where both values are verbs that >> answer "what action?". This makes the interface clearer. >> >> **Aligned config name with command-line option** >> >> Changed replay.defaultAction to replay.refAction. The config variable >> now mirrors the option name, making the relationship obvious. >> >> **Unified config and command-line values** > I didn't see anything glaringly wrong in this round, even though I > picked a couple of small nits in one patch, so we might want a > hopefully small and final reroll before marking the topic for > 'next'.
Thanks! I will address all the feedback from you, Christian, and Phillip in v5:
- Add trailing comma to enum definition - Fix error message quoting with single quotes - Revert the `const char * const` formatting change - Follow standard Git config pattern (repo_config before parse_options) - Extract proper helper functions for string-to-enum conversion - Switch to `test_grep` and `test_config` in tests - Fix documentation wording issues
Should have v5 ready soon with these fixes.
Thanks, Siddharth
> > Is everybody else happy with this iteration otherwise? > > Thanks.