"Jayati Shrivastava via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: Jayati Shrivastava <gaurijove@xxxxxxxxx> > > 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.
