Just a quick reply, will be away from home for a few days.
On 26-3-2014 4:56, Nathan Hurst wrote:
On Tue, Mar 25, 2014 at 11:39:51PM -0400, Martin Owens wrote:
As for policy regarding commit access; we don't do enough to encourage code review. Anything to encourage code review would be great, I don't even mind if I were downgraded to B level 'code must be reviewed before merge' status. Because really, it should be.
I like this. I'll even put my hand up to do some code reviews. Can we just turn code reviews on with our current tooling?
I think we lack the resources for code reviewing everything, at least in minute detail. Note that patches very often go unreviewed for a long time, and I don't think that is just because they go unnoticed by a potential reviewer. This is the reason I am pushing for "automated review", in the form of -Werror(*) and static analyzers, and possibly automated code pretty-printing by clang-format. This way, at least we can focus our review on more design-like choices, and not get bogged down in reasoning about hard-to-read and/or easily-bugged code.
I try to look at every commit, and I believe some others do too. But it becomes tiring when simple things are 'wrong' that would have been easily caught by e.g. -Werror.
regards, Johan
(*) please think with me. I mean -Werror with all the improvements as discussed on maillist, for example disabling some errors for certain files.