On Nov 25, 2013, at 9:17 PM, Alan Modra <amo...@gmail.com> wrote: > Was Re: [buildrobot] [PATCH] mips: Really remove ENTRY_BLOCK_PTR > On Wed, Nov 20, 2013 at 10:08:45AM +0100, Steven Bosscher wrote: >> This patch is obvious and it fixes breakage. Please go ahead and commit it. > > Sorry to pick on you here Steven, but this doesn't meet gcc's > definition of an obvious patch. Don't believe me?
No. I don't, let me quote from the policy: We don't want to get overly anal-retentive about checkin policies. Nit picking the fine lines from a lawyer perspective as to the exact meaning of the word is, is best suited for lawyers and presidents. We are above that around here. We work through co-operation and a shared desire to push gcc in the direction of being the best it can be. The more rules one writes about what is and is not acceptable and the more capriciousness one uses when applying those rules, the worse off I think we are. I don't favor that direction. Merely mentioning that a fix that is checked in to resolve a build issue doesn't meet your understanding of the letter of the law, I think is already takes us in a non-desirable direction. You discourage lots of people by making them have second thoughts about checking in any fix to any build issue. I personally favor a compiler that builds. If you have a substantive disagreement as to the exact formation of the patch that was checked in under the obvious rule, I think the right approach is to point out the issues that you have with the patch and if you feel strongly enough that the source tree is better without it, to ask for the person to revert it (or fix it to more your liking). In the end, we can trust that a reviewer will settle any disagreements. They can approve the patch as is, insist that it be removed, explain how it should be fixed instead… If we change the text of the policy, I think we should resist the temptation to make it more strict. We can explain that patches checked in under the obvious rule are free to be post-checkin reviewed by a reviewer and that review can include any/all of the usual review responses and can include a please revert. If anything, we should list more categories of what obvious is, or say, including, but not limited to, so that people don't form an overly restrictive view of the policy as you have done. A example of a category not listed is unbreaking the build. > It's utterly ridiculous that gcc doesn't have a reasonable obvious > patch rule. Maybe we already do, and you never knew it. :-) I already have a reasonable obvious patch rule. > Only comments? In the parts I review for, the rule isn't restricted to comments. The history of gcc is replete with examples that are non-comment changes under the obvious rule and the maintainers of those areas that think, at least in part, as I do as well.
