On Sun, Apr 25, 2010 at 3:00 PM, Richard Kenner
<ken...@vlsi1.ultra.nyu.edu> wrote:
>> Eliminate the easy mistakes in patches. GCC uses strict coding conventions,
>> including formatting and commenting conventions, so not following them is a
>> mistake that will be flagged as such. Fortunately this is easy to correct,
>> you don't even need to read the (whole) documentation, just look around in
>> the existing code you're modifying and make it so that the new code cannot
>> be distinguished from the old one in this respect.
>>
>> Write proper ChangeLogs. They are kind of executive summaries for patches
>> and help to grasp what they do. The various ChangeLog files have many
>> examples.
>
> Moreover, I think that having a patch that's improperly formatted or
> missing or improper ChangeLog may simply cause reviewers to ignore the
> patch because they don't want to have to deal with explaining these
> things to people. This SHOULDN'T happen, but I'm pretty sure it does.
In the aerospace industry we have a somewhat similar problem. Every
part of assembly drawing has to be reviewed ("checked") and approved.
But the common mistakes often are so distracting that checkers don't
even want to begin to comment on to-be-released drawing in the PDM
system. It is very demotivating to review something and you're just
pointing out issues with formalities instead of focusing on the actual
design.
The solution for this in my working environment is an automatic
checker. This checker validates the drawing against a set of
requirements (proper design principles, proper drawing formatting,
correct label, etc.). You can't upload a drawing for check in the PDM
system until it passes the checker (it is part of the PDM system, and
it simply rejects drawings that don't pass).
Perhaps it is possible to create some kind of check-patch script for
GCC too, e.g. one that checks the following things at least:
* ChangeLog presence
* ChangeLog format and completeness
* formatting / coding style of the patch itself
We should perhaps make tools available in contrib/ that help people
set up things properly for patch submission. For example Diego had a
script that extracts a skeleton ChangeLog from a patch, perhaps it
should be put in contrib/ and advertised somewhere (e.g. wiki).
There are many things beside this that we could do to simplify the
patch submission process. It's just part of the problem but perhaps it
helps.
Ciao!
Steven
