Re: [PATCH 05/15] ref-filter: abstract ref format into its own struct

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> On Thu, Jul 13, 2017 at 8:01 AM, Jeff King <peff@peff.net> wrote:
>
>>  builtin/branch.c       | 14 +++++++-------
>>  builtin/for-each-ref.c | 22 ++++++++++++----------
>>  builtin/tag.c          | 30 ++++++++++++++++--------------
>>  builtin/verify-tag.c   | 12 ++++++------
>>  ref-filter.c           | 22 ++++++++++++----------
>>  ref-filter.h           | 22 +++++++++++++++++-----
>>  6 files changed, 70 insertions(+), 52 deletions(-)
>
> The patch looks good to me. So some off-topic comments:
> I reviewed this patch from bottom up, i.e. I started looking at
> ref-filter.h, then  ref-filter.c and then the rest. If only you had formatted
> the patches with an orderfile. ;)

As a reviewer, for this particular patchq, I actually appreciated
that ref-filter.[ch] came at the end.  That forced me to think.

When I see something that used to be 0 is lost from the parameter
list of show_ref_array_item() at a callsite, I was forced to guess
what it is by looking at what is moved into the new structure
nearby, and keep doing that "figure out what is going on" game until
the "author's answer" is revealed at the end of the patch.  

By the time I reached ref-filter.[ch], I had a pretty good idea what
was done by only looking at the callers and being able to see if my
understanding matched the "author's answer" at the end of the patch
turned out to be a very good way to double-check if the patch was
sane.  If I were given the author's answer upfront, especially an
answer by somebody as competent as Peff, I'm sure I would have been
swayed into believing that whatever is written in the patch must be
correct without thinking the changes needed in the patch myself.

I do want to present from Doc to header to code when I am showing my
patch to others, so this is probably a good example that illustrates
that the preferred presentation order is not just personal
preference, but is different on occasion even for the same person.