[0/2] ref-filter: merged & no-merged touch-ups

Message

Aaron Lipman Sept. 18, 2020, 12:49 a.m. UTC

> > Even if we are not renaming
> > things that much, a locally defined preprocessor macro with shorter
> > names, defined just before the callee, would be more appropriate for
> > a case like this, with a single callee called by only one caller.

> Thanks for stating what I was planning on saying, with particular
> emphasis on keeping these #defines in the .c file rather than the .h
> file since they are not part of the public API.

Thanks for the insight into defining macros in source vs header files-
I'm still in the process of learning C and figuring out how to convey
purpose & encapsulation without relying on features of higher order
languages I'm used to (e.g. keyword args).

Since this got moved to next, I'm not entirely clear on whether folks
want me to keep tweaking this, or if the last set of comments were
general advice for future contributions.

But in the interest of diligence, I'd like to offer a couple touch-ups
applying Junio's suggestions.

Aaron Lipman (2):
  ref-filter: refactor do_merge_filter()
  Doc: prefer more specific file name

 Documentation/git-branch.txt                  |  2 +-
 Documentation/git-for-each-ref.txt            |  2 +-
 Documentation/git-tag.txt                     |  2 +-
 ...lters.txt => ref-reachability-filters.txt} |  0
 ref-filter.c                                  | 29 +++++++++----------
 ref-filter.h                                  |  3 --
 6 files changed, 17 insertions(+), 21 deletions(-)
 rename Documentation/{filters.txt => ref-reachability-filters.txt} (100%)

Comments

Eric Sunshine Sept. 18, 2020, 2:52 a.m. UTC | #1

On Thu, Sep 17, 2020 at 8:49 PM Aaron Lipman <[e-mail]> wrote:
> Since this got moved to next, I'm not entirely clear on whether folks
> want me to keep tweaking this, or if the last set of comments were
> general advice for future contributions.
>
> But in the interest of diligence, I'd like to offer a couple touch-ups
> applying Junio's suggestions.

In this case, the review comments were reasonably important, so
tweaking it by patching what has already been moved to 'next', as you
do here, is a good choice. Thanks for the diligence. I'll leave one or
two minor comments on the patches themselves (although, it's probably
not worth re-rolling just for them).