John Keeping wrote:
> On Sun, May 19, 2013 at 12:36:12PM -0700, Jonathan Nieder wrote:
>> Who is responsible for freeing
>> "path"? Would it make sense to use a strbuf here?
>> strbuf_setlen(&buf, traverse_path_len(info, n));
>> make_traverse_path(&buf.buf, info, n);
> Perhaps alloc_traverse_path? I'm not sure the strbuf buys us much for
> this case, since we only use the result for a few lines before freeing
Good idea. Generally strbufs are nice for this kind of thing because
they make it easy to reuse buffers, but in this case there's no
opportunity for that.
> then this is "process_changed_path".
>> What should happen for commits with more than 1 parent? If they're
>> supposed to not enter this codepath because of a previous check, a
>> die("BUG: ...") could be a good way to make reading easier.
> Currently the patch ID code compares the commit with its first parent,
> so I think this should do the same.
Ok. I guess a comment nearby would help future readers avoid the same
I don't know what it should mean to try to use --cherry without
--no-merges or --first-parent, so I guess this is harmless.
> Thanks for the review.
No problem. Thanks for a pleasant read.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html