[LLVMdev] LLVM maintainers, code reviews

[Renato Golin]

On 10 November 2010 07:27, Chris Lattner <clattner at apple.com> wrote:
> I'd like to do more similar ones in the future, and encourage other contributors to also write other blog entries in general.

Hi Chris,

After discussing these topics in the dev meeting, I also have some input.

First, blog posts are great for communicating changes and maybe
outlining design decisions, but these decisions change over time, and
the posts become obsolete. On the other hand, HTML documents, while
good for release process, are hard to change and keep it up-to-date.

As I said before, I'm looking for a (or writing a new) script to
convert from the Wiki to LLVM-HTML style, and I think we should use
the Wiki more often to keep not only the documents up-to-date, but
also the design decisions, libraries and how-tos.


> I personally think that there are a several key aspects of our development process that contribute to ensuring that LLVM maintains its high quality over time:
>
> 1) peer review (review after commit is enough if people are willing to revert and if people actually do read the patches and speak up).

This process can take months to apply and generally impact on the
productivity of the team that sent the patch in the first place. We
had a few cases where the patch was never looked upon until someone
hit the same problem and happened to find it in the llvm-commit list.

Also, without a clear design roadmap for the various parts of LLVM, it
falls too much in the hands of the reviewers to check that everything
is correct, interact with the developer, explain the design decisions
(or not, and complicate things even more).

Peer review is fundamental, but itself alone cannot deal with a higher
volume of patches, probably not even with the current volume, and
increasing the number of committers can have other unexpected side
effects in code quality (they themselves may not know all design
decisions).


> 3) components that are not on-by-default can be held to lower standards while they are being implemented.  If they work out and it seems like a good idea to turn them on by default, they should be given a really hard once-over.
> 4) continuous refactoring and improvement of the code base
> 5) being willing to throw away old stuff that no one is using or maintaining, or replace things that are broken and terrible.

IMHO, these are the three key points that are maintaining the high
quality of LLVM and should be kept forever. As much as I disliked
having the union type removed, it was a good design decision.

The same should go for back-ends. It's good to have a long list of
supported back-ends, but if they start hampering the evolution of the
rest, they should be turned off by default or even moved to a separate
patch/project outside the main distribution.


In a nutshell, the design decisions should be communicated more
effectively, and a Wiki is a great place to start. Peer reviewers
should communicate via the Wiki, so patchers could learn and plan
before the next iteration and reduce the cost for everybody.

cheers,
--renato