Re: [PATCH v3] sequencer.c: use reverse_commit_list() helper

[Junio C Hamano <gitster@pobox.com>]

"Jayati Shrivastava via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Jayati Shrivastava <gaurijove@gmail.com>
>
> Instead of creating a new allocation, reverse the
> original list in-place by calling the
> reverse_commit_list() helper.

OK.  I recall that v2 wrapped with longer lines that was easier to
read.

> The original code
> discards the list "bases" after storing its
> reverse copy in a newly created list "reversed".
> If the code that followed from here used both
> "bases" and "reversed", the modification would not
> have worked, but since the original list "bases"
> gets discarded, we can simply reverse "bases"
> in-place with the reverse_commit_list() helper and
> reuse the same variable in the code that follows.

I am 30% surprised to see this in the log message ;-).  There is
nothing incorrect in the description per-se, but it is something I
would expect in a review that reads a patch and follows along
thinking aloud to see if the code makes sense, or below "---", but
it does explain why the patch chose to lose the extra variable, so
probably it is OK.

> builtin/merge.c has been left unmodified, since in its case, the
> original list is needed separately from its reverse copy by the
> code.

Good.

The patch text looks good, too.

Thanks.