Re: [PATCH 1/5] grep: perform some minor code and comment cleanups

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

Eric Sunshine <sunshine@sunshineco.com> writes:

> It's entirely subjective, of course, so no right-or-wrong answer, but
> I personally do not find that this change improves code quality or
> readability.

I agree that this is entirely subjective.  To those who wrote these
variable decls and inits, what they wrote was the most readable,
wasn't it?  It probably falls into the "to some readers the existing
code may not be perfect, but once it is written, it is not worth a
patch noise to fix it" category.

> With my reviewer hat on, I spent an inordinate amount of time staring
> at this change trying to locate each variable's new location to verify
> that no initializers were dropped and that the declared type hadn't
> changed.

It is true that "cleaning up, no behaviour changes intended" patches
are unpleasant to review.  They are boring to read, and the risk of
breakage due to mistake is unnecessary and severe.

But if the result is objectively better, such a one-time cost may be
worth it.  We are investing into the better future.  For example, we
may have an unsorted mess of an enum definition, and we do
appreciate in the longer run, such a definition were "more or less"
sorted within the constraint of some other criteria (like, "errors
get negative value").  If the enum is a huge one, it may need some
careful reviewing to verify such a change that turns the unsorted
mess into a sorted nice list, but the cost of doing so may be
justified.

Does the change in this patch qualify as "objectively better"?  I
dunno.

Thanks.