Re: [PATCH v10] show-branch: convert per-branch flags to commit-slab
Gatla Vishweshwar Reddy <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
While waiting for your signal to send the next version, I reviewed
the patch myself and found a few issues I want to address:
1. show_merge_base() has inconsistent indentation on the continuation
line — has_all_rev_flags() is indented with a tab instead of
spaces to align with the opening parenthesis. Should be:
if (!(commit->object.flags & UNINTERESTING) &&
has_all_rev_flags(commit, num_rev)) {
2. int _b is declared in the middle of the parents loop body in
join_revs(). It should be declared at the top of that scope.
3. Regarding the {} block in join_revs() — as I explained earlier,
it scopes commit_is_merge_base after mark_seen(). I can move the
declaration to the top of the while loop body and remove the {}
if you prefer that style.
4. Regarding test coverage — your footnote mentioned a gap since the
test suite missed the basic crash. Should I add a test that simply
runs git show-branch on two branches as a smoke test before sending
the next version?
Please let me know what other issues you spotted and what signal
you need from me before I send the next version.
Vishweshwar