Re: [PATCH] config: Use parseopt.

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

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> I personally do not think "I rewrote this command's option parser using
>> parseopt" earns any "trust point".  I think the latter is a *great* thing
>> to do, though.
>
> I disagree. Making a patch pass through all the filters must mean
> something, and the more patches the more trust.

Why are you arguing?

I am saying I do not feel more trust in people _merely because_ they
rewrote command parser to use parseopt.  Telling me that I am wrong and I
should trust you more for such a patch would not help.

>> Mistakes made in the past and resulting flaws that remain in the current
>> source do not justify a new patch adding more mistakes to the codebase.
>> Reviewers help the author from adding more.
>>
>> One bad thing about the current process (and I am certainly one of the
>> guilty parties because I do a lot of reviews) around this area is that a
>> review comment that points out a mistake similar to the ones made in the
>> past sound to the author of the patch as if the reviewer is telling that
>> the patch will not be accepted unless the existing mistakes are also fixed
>> by the patch author.  Such a review certainly does not mean that ...
>> ...
> But, if there's code that already has the same issues the patch has,
> it doesn't look like a good argument for patch rejection. Perhaps the
> quality standards increased, but on the other hand things wouldn't get
> worst by applying the patch.

If you read the above quoted block again, you will notice that we are
almost in full agreement, *if* you change "by applying the patch" in your
last sentence to "by applying a patch that is revised to fix the problem
pointed out during the review in it, even if it does not address the
similar ones in the existing code".

Adding more breakages of the same kind may not increase the number of
classes of breakages, but surely it increases the number of places that
need to be fixed later, and it is actively wrong to discard time and
energy somebody already spent to prevent one more instance of known
breakage to get into the codebase.