Re: [PATCH v4 00/12] ref-filter: use parsing functions

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 5, 2016, 00:34 UTC
Message-ID: <CAPig+cRr3UTfcegcw6L6qXr7TFaCw2Uno696q6Kfkttp97fGRg@mail.gmail.com>
In-Reply-To: <xmqqr3gwt6dp.fsf@gitster.mtv.corp.google.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 20 quoted lines

> This series cleans up populate_value() in ref-filter, by moving out
> the parsing part of atoms to separate parsing functions. This ensures
> that parsing is only done once and also improves the modularity of the
> code.
>
> v1: http://thread.gmane.org/gmane.comp.version-control.git/281180
> v2: http://thread.gmane.org/gmane.comp.version-control.git/282563
> v3: http://thread.gmane.org/gmane.comp.version-control.git/283350
>
> Changes:
> * The parsing functions now take the arguments of the atom as
> function parameteres, instead of parsing it inside the fucntion.
> * Rebased on top of pu:jk/list-tag-2.7-regression
> * In strbuf use a copylen variable rather than using multiplication
> to perform a logical operation.
> * Code movement for easier review and general improvement.
> * Use COLOR_MAXLEN as the maximum size for the color variable.
> * Small code changes.
> * Documentation changes.
> * Fixed incorrect style of test (t6302).

v4 is a nice improvement. With the retirement of match_atom_name() and its misleading and confusing use in the parsers, overall the parsers are now more concise, straightforward, and easier (in fact, dead simple) to comprehend.

As most of my review comments this round were relatively minor, and there don't seem to be any major problems, hopefully this series will wrap up with v5. Thanks.