Hello,
I have read the mail of Steven Bosscher
(http://gcc.gnu.org/ml/gcc-patches/2006-05/msg00272.html), and while IMO
he takes the situation too personally, he raises several interesting
points. In particular, some of the statements remind me of my own
experiences with the work on loop optimizer I spent last few years with.
As you probably noticed, we managed to replace loop.c finally; however,
I feel that if we did not have to rely on "luck" of finding a willing
reviewer for each patch, this work could have been finished at least one
release earlier, and additionally, the loop optimizer would be in much
better shape.
I have spent some time thinking about this situation, and I would like
to share some thoughts with you. I am still quite far from concrete
proposals, but I would like to hear your ideas.
Consider a person or a group of persons without write privileges working
on some larger project. At the moment, the only thing they can do is to
create a development branch, spend some time working there (possibly
posting some progress reports and/or asking other people for suggestions
and help), and finally get their work merged. At this point, the
problems begin. There are basically two ways how to get things done:
They may try to get everything merged at once. This usually does not
work, as nobody is willing to review a nontrivial patch whose size
exceeds a few thousands of lines, especially if it has to touch several
separate parts of the compiler (and there are many other good reasons
for avoiding this).
So, they have to resort to splitting the merge into several parts. This
however brings more problems.
-- it is quite time comsuming. There are usually some dependences
between the patches. It would be nice to be able to submit all the
patches at once, however, creating and testing the patches that
depend on other patches is quite nontrivial and with more patches, it
gets close to impossible. So one has to submit the patches step by
step, and wait each time till someone reviews the patch; if one patch
in long dependency chain gets stuck, it is a disaster.
-- the reviewer lacks the broad picture (does not exactly know what
other patches depend on this one). If one of the patches makes a
change that by itself is not very appealing, it is difficult to find
a reviewer for it.
-- the reviewer may request changes that are incompatible with the
followup patches, or with the general idea of the project. In such
case, changes have to be made during the merge, which makes it much
more likely to introduce bugs, and makes the previous testing less
relevant.
-- the worst-case (that never happened to me, but I felt dangerously
close to it several times) is that reviewer requests changes which
are totally incompatible with the rest of the project, or which the
authors of the project consider wrong, and he does not yield to
argument (for example, because he believes a completely different
approach should be taken). In such case, it is nearly impossible to
find a different reviewer for the patch and proceed in any way,
except for yielding yourself and doing something one believes to be
wrong.
-- the patches usually get reviewed by several reviewers. Each of them
has different opinions and the changes requested by them may be
inconsistent. Additionally, this is inefficient for the reviewers as
well -- each of them should understand the project and the
relationship of the patch to it. IMO this gets neglected sometimes,
the patches are considered in isolation and the reviewer simply
trusts the author that the relationship of the patch to the rest of
the project is sane (for example, that the implemented algorithms are
appropriate for the task they are used to).
Obviously, there is no perfect solution for this problem. However, here
are several ideas for proposals that I believe could help:
1) Every large optimizer or major piece of the compiler should have a
dedicated maintainer. At the moment, maintainers are assigned on an
ad-hoc basis. Some parts of the compiler have a proper maintainer.
Some have an "implicit" maintainer (e.g., everyone knows that when
something breaks in DOM, Jeff is the right person to speak with, but
it is not written anywhere). Some parts have an "implicit"
maintainer that however does not have right to approve the changes to
the code (e.g., if something breaks in ivopts or # of iterations
analysis, everyone knows that I am to be blamed :-). Some parts lack
the maintainer completely (for example, the loop optimizer as whole
does not have a dedicated person, and the patches there are approved
by anyone who just happens to have time).
Proposal: Whenever a new pass or a major functionality is added to
gcc, a maintainer for it must be found. Preferably the
author, or in case he from some reason is not considered
suitable, some other person must be assigned.
(? Should this person also automatically have right to
approve changes to this code? While this is
recommendable, in some cases it might be enough if this
person would be here to "advice" the reviewers and comment
on suitability of the changes.)
2) Even if the optimizer has a maintainer assigned, such a maintainer
often is not active; either he stopped the work on gcc completely, or
works on something completely different now, or "his" component might
pass through so many changes that he no longer is the appropriate
person to maintain it, or it might not really exist any more (we
happen to have a maintainer for "jump.c", which at the moment
consists of several utility functions).
Proposal: Maintainership should not be considered only right, but
also a duty. By accepting the maintainer rights, the
person also accepts the obligation to take care of the
assigned component, reviewing relevant patches and ensuring
that the bugs relating to that component are fixed (not
necessarily fixing them by himself!). If the maintainer is
no longer able to perform these duties from any reason, he
should resign on them and another person has to be found,
preferably with the help and consent of the old maintainer.
3) Every major project should be overseen by an experienced person, that
will ensure that all the changes done in the project are reasonable
and according to the regulations of gcc development, and makes the
process of merging the changes more fluent.
Proposal: Whenever a new development branch is created, the author(s)
should persuade a person with right to approve at least the
major parts of the project developed on the branch to
oversee it; preferably the maintainer of the affected
component. This person should be kept informed about the
progress of the project, and the project should not be
merged until he agrees that the code is ready, and that
that the changes were tested sufficiently. He is also
responsible for reviewing the patches for that he has such
right.
(? I think this should be formulated as a suggestion, not strict
regulation, as there are special situations under that this proposal
is not reasonable).
Zdenek
