From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney To: GDB Discussion Cc: GDB Patches Subject: The ``obvious fix'' rule. Date: Tue, 16 Jan 2001 16:59:00 -0000 Message-id: <3A64EDE9.74ADADAF@cygnus.com> X-SW-Source: 2001-01/msg00104.html [*NOTE* Reply-to: set to gdb@sources.redhat.com. Reply-to is bad M'kay] (just moving a discussion on gdb-patches across to the discussion group) Hmm, I looked at one of the postings and it occurred to me that yes it was probably a good time to discuss this one. - January is always a slow month. Looks like the boat left without me :-) Anyway, yes, at present GDB doesn't have an obvious check in rule (although some people have probably been sneakering in changes in the basis that it does :-). Instead, people listed under the blanket write privs list are free to check in anything. If something is really really obvious then, they'll just check it in without reference to other maintainers. In general though, the blanket write privs maintainers go out of their way to not step on the toes of the maintainers of specific areas. I've observed on several occasions that getting onto the Blanket Write Privs list gives you less not more freedom :-) I think the recent problem with a C++ related core dump is probably more of an example of an unfortunate sequence of events than justification to introduce an obvious fix rule. Maintainers can't veto for fail to respond to a patch because they are just about, real soon now, like the cheque is in the e-mail, just getting it out the door, as we speak, going to post the worlds greatest rewrite of a given piece of code. Instead maintainers need to allow people to incrementally improve the existing code base. For me personally, juggling this is really hard - I have to ask the question: is this change, while not perfect, still taking things in the right direction - does it take things forward more than sideways? There are two things (more?) that I do tend to dig my heels in with though: interfaces and coding standards. In both cases it is because I think GDB is littered with examples of where standards were dropped and now we are still paying the price for dropping our guard :-(. For interfaces, I think it is important to put significant effort in trying to get them right. It is the interfaces and not the implementation that is going to last for very long time. With coding standards, I've two concerns: bad code has this nasty habit of breeding (believe me :-) and I've a strong preference for keeping code as readable (dumb it down) as possible. Many many many many many many many more people than the author are going to need to read/understand that code. (Any way, returning to your regular program) So what about an obvious fix rule? To show my colours (note spelling :-) I've found that too often what is claimed to be an obvious fix is unfortunately wrong. Rather than fixing a problem it just hide it, or worse, the patch will often take the the code base in directions it just shouldn't (the twilight zone of maintainability). A favourite obvious fix involves bypassing interfaces and grubbing around in internals (ex registers[]). The thing that will really get up my nose is someone making a change, and then announcing it after the fact with the claim it is an obvious fix :-) Against all that, there is an obvious needs to further loosen those purse strings: o GDB must compile syntax errors and configury problems o GDB must not dump core o GDB must meet coding standards (you can all roll around and laugh at this one :-) o others? Must compile - syntax errors: I guess this has to go into the obvious fix with notification after the event category. After all, this will never occur will it. Must compile - configury changes: People are already free to tweak Makefile.in and configure.in. Just ask for comment first. Extending it to other files (adding autoconf stuff) makes sense and I suspect people have already being doing that :-) Must not dump core: more tricky. Since there is now a gdb_assert() macro, I'm very tempted to suggest that the obvious fix to a core dump is to add ``gdb_assert (dud_pointer != NULL)'' :-) Beyond that, things typically do need review. Flame away. Must meet coding standards - spelling: This one has already become an obvious fix rule. But remember those cultural sensitivities. Must meet coding standards - style: At present developers get a licence to eradicate things (cf Kevin's work). To be honest, I think this is working. It means that code improvement maintaining a certain pace without getting out of control. It also means that we're not spending all our time fixing non- problems :-) Other thoughts? Andrew