On Wed, Sep 16, 2020 at 12:53 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Even though I said "I do not mind", using DO_MERGE_FILTER prefix is
> way too much noise. The common prefix is appropriate for external
> API, but this is merely an internal contract between a private
> helper do_merge_filter() and its only caller.
>
> #define EXCLUDE_REACHED 0
> #define INCLUDE_REACHED 1
> static void reach_filter(struct ref_array *array,
> struct commit_list *check_reachable,
> int include_reached) {
>
> to make it clear which part of the callback struct is really passed
> between the caller and the helper. 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.
Aside from that, this re-roll seems to address all of my previous
review comments. Thanks.
