On Sat, Feb 14, 2009 at 10:35 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Felipe Contreras <felipe.contreras@xxxxxxxxx> 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. I'm not arguing, that's just my opinion. I'm not saying writing a patch should give anyone penguin points, it's going through the review process up to patch acceptance. Maybe not to you, but maybe for other developers. >>> 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. I think are in full agreement, it's just that I wasn't clear enough; those are not the kind of issues I was talking about. If there's a known way how to do something then it's obvious the patch should be re-submitted with the 'right way'. I was thinking more on "FILE" vs "file", or any kind of issue that would require a separate patch to reach consistency in the existing code; those can wait until after the original patch is accepted. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
