[PATCH] Include a clearer policy about what's ok/nok to speed up code reviews.

David Tweed david.tweed at arm.com
Fri Aug 23 10:12:08 PDT 2013


Hi,

| The other problem I see is that the "approvers" can usually just
| commit their own patches without having to go through the pre-commit
review
| process.  So, it is difficult to build goodwill with them since there is
| usually very little opportunity to review their patches.  I know there
| is always the opportunity for post-commit review, but the goodwill comes
| from taking the time to review somebody's patch and helping them get it
| into TOT quickly, as opposed to them having to wait a week or two to
| commit.

I thought that, while people may have the ability to commit without
pre-commit review, even then it was discouraged to do that except
in the case that the patches were either urgent (fixing an issue caused
by an earlier check-in), or simple enough to be certain to be OK. Granted
some people differ in compiler programming skills so that what one person
might
consider to be simple enough is something another compiler programmer
wouldn't
consider simple enough (and to be clear, I don't put myself at the
expert end of that compiler programming scale). However even given that
I thought it was policy that all significant patches, regardless of the
author
and their ability to commit directly, should go through some review before
being checked in.

If I were to focus on a community difficulty (in addition to the one of
latency reviewing
newcomers patches that prompted the documentation change), it's that there
are so many avenues
for communication about patches in the LLVM community (email, IRC,
phabricator,
people who physically work together looking at stuff non-electronically)
that if you only follow one (or maybe two) it can be easy to miss out on
stuff that
you could review (or a review chain one could usefully participate in) while
the patch is still in the review stage. However, I don't have any
silver-bullet
suggestions for this...

Cheers,
Dave






More information about the llvm-commits mailing list