Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
> Thanks for the new version, this is looking pretty good now, just a
> few comments below
I agree that this step is looking pretty good now.
I didn't check closely, but when 1/3 undergoes necessary polishing,
it may have repercussions on this step, though (I did see that the
change in 3/3 would have overlaps with what was touched by 1/3 that
needs to be done differently).
Thanks for guiding Rohit's series forward.
> This is only slightly different from reset_for_rollback() if you
> decide to keep a separate code path for skip vs abort then I'd be
> tempted to combine the two like this.
>
> diff --git a/sequencer.c b/sequencer.c
> index ecf4be7e15..b187b4222e 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2740,11 +2740,13 @@ static int reset_merge(void) {
> static int reset_for_rollback(const struct object_id *oid)
> {
> const char *argv[4]; /* reset --merge <arg> + NULL */
> + size_t i = 0;
That size_t is, eh, "unusual". For an index into a small local
array of known size, just sticking to bog-standard-and-boring 'int'
would make it less distracting for future readers of the code.
Or even better, perhaps use argv-array, so that you do not have to
worry about sizing the local array sufficiently large in the first
place.
> + argv[i++] = "reset";
> + argv[i++] = "--merge";
> + if (oid)
> + argv[i++] = oid_to_hex(oid);
> + argv[i] = NULL;
> return run_command_v_opt(argv, RUN_GIT_CMD);
> }
