[Clfs-dev] Code Review Tools

[Ken Moffat]

On Thu, Oct 23, 2008 at 11:12:39AM +0300, George Makrydakis wrote:
> Hi list,
> 
> I have been doing some evaluation of modern code review tools. Could it be 
> fruitful for clfs to have one for the users that are actively editing the 
> book together? Some summary links about this are:
> 
> http://ostatic.com/161492-blog/open-source-code-review-tools
> 
> http://google-code-updates.blogspot.com/2008/07/looks-good-to-me-source-code-review.html
> 
> Just thinking that it could be of use when editing files together or trying to 
> find bugs and audit book / code in a more efficient manner, opinions?
> 
> George

 Hi George,

 to me those are all tools geared at reviewing source code.  The
nearest an editor gets to altering source code is in writing a patch
to make something work.

 As an editor, my view of bugs in clfs is that they fall into the
following categories:

1. A book instruction is wrong in general, or an explanation is
awkward. Yes, somebody reviewing the book might catch these.  If the
instruction is broken, people trying to build it will catch this.

2. An instruction doesn't work on a particular architecture.  In my
experience, these only get caught when someone tries to build on
that architecture.  Often, these problems are caused by an unrelated
change (i.e. something used to work, but upgrading a different
package has broken it).

3. We are using a package with known vulnerabilities, but without
addressing them.  But, I might be on my own in caring about this.

4. The system builds and boots, but it has problems with a
particular package further down the line.  When we were maintaining
our own headers, there was a lot of this.  Nowadays, the problems
are usually fixed by kicking the tyres of the affected package
(patch, sed, whatever).


 For the xml in the book, I don't think "code review" either happens
or is necessary.  Provided the book validates and renders, that
should be enough.  Sometimes, people might make stylistic comments
but they can do that do that from the rendered book.  Sometimes, we
change something and accidentally replace an instruction for another
architecture (I know I've done this).  Anyone who looks at the
commits can review them, but these problems are usually less than
obvious - they're one of the downsides to how clfs is marked up.

 Where people are writing code, code review is good.  In my opinion,
what clfs needs is more people to use it and to comment on it,
particularly for non-x86_64 (even x86 appears to be a minority
interest here).

ĸen
-- 
das eine Mal als Tragödie, das andere Mal als Farce