[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: A more hands on review process
From:       Alex Merry <huntedhacker () tiscali ! co ! uk>
Date:       2008-08-01 10:03:57
Message-ID: 200808011104.02461.huntedhacker () tiscali ! co ! uk
[Download RAW message or body]


On Thursday 31 July 2008 19:29:54 Stephen Kelly wrote:
> [snip] This could make the review process shorter,
> at the expense of the developer putting in more effort to document the
> app/lib.

In my personal opinion, this is no bad thing...

In general, +1 from me, but:

> Style
> * Coding style in consistent with the rest of the library / application
> * Variable names are consistent with the target module or library module.
> (m or m_ prefix, camel casing, descriptive variable names instead of a, b
> and c2 etc)
> * There are no excessively large methods. Methods longer than 40 lines can
> be broken into multiple methods if possible.
> * There are no excessively long lines. for lines greater than 80 characters
> it might be possible to use intermediate variables more.

I think this is where you're going to get issues.  If the target module has a 
coding style (eg: kdelibs or kdepim), then the application or library should 
conform to it, sure.  But I don't see why (and I suspect neither will authors 
of code put in to kdereview) they should follow a coding style just because a 
lot of others in the same module use it.

Note that most of the code going into kdereview is not going to form part of 
an application or library (since such additions usually have their own review 
processes), but be an application or library in itself.

I can see that if the reviewed code is, say, a plugin for kopete or a data 
engine or applet for plasma due for inclusion into kdebase, for example, you 
might want to follow the coding style of that application, but I think it 
should be down to the maintainers of the application as to whether to enforce 
that.

Alex


-- 
KDE: http://www.kde.org
Ubuntu/Kubuntu: http://www.ubuntu.org http://www.kubuntu.org


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]