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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.