A more hands on review process

[Alex Merry]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080801/7a35b5de/attachment.sig>