Re: [PATCH v6 3/3] ident: cleanup wrt ident's source
- From
Jeff King <peff@peff.net>
- Date
- Feb 5, 2016, 19:24 UTC
- Message-ID
- <20160205192413.GB7245@sigill.intra.peff.net>
- In-Reply-To
- <xmqqsi17c70w.fsf@gitster.mtv.corp.google.com>
On Fri, Feb 05, 2016 at 11:05:19AM -0800, Junio C Hamano wrote:
Show 20 quoted lines
> Dan Aloni <alonid@gmail.com> writes:
>
> > This change condenses the variables that tells where we got the user's
> > ident into single enum, instead of a collection of booleans.
> >
> > In addtion, also have {committer,author}_ident_sufficiently_given
> > directly probe the environment and the afformentioned enum instead of
> > relying on git_{committer,author}_info to do so.
> >
> > Signed-off-by: Dan Aloni <alonid@gmail.com>
> > Helped-by: Jeff King <peff@peff.net>
> > Helped-by: Junio C Hamano <gitster@pobox.com>
> > ---
> > ident.c | 126 ++++++++++++++++++++++++++++++++++++++++------------------------
> > 1 file changed, 80 insertions(+), 46 deletions(-)
>
> Peff what do you think? I am asking you because personally I do not
> find this particularly easier to read than the original, but since
> you stared at the code around here recently much longer than I did
> when doing the 1/3, I thought you may be a better judge than I am.I'm not sure it is really worth it unless we are going to expose this to the user, and let them say "I am OK with IDENT_SOURCE_GUESSED, but not IDENT_SOURCE_GUESSED_BOGUS" or similar.
Without that, I think it is probably just making things a bit more brittle.
-Peff