Re: share/private/slave a subtree - define vs enum

From: Horst von Brand
Date: Mon Jul 11 2005 - 12:28:41 EST


Vojtech Pavlik <vojtech@xxxxxxx> wrote:
> On Sun, Jul 10, 2005 at 09:21:42PM +0300, Pekka Enberg wrote:
> > Hmm. So we disagree on that issue as well. I think the point of review
> > is to improve code and help others conform with the existing coding
> > style which is why I find it strange that you're suggesting me to limit
> > my comments to a subset you agree with.
> >
> > Would you please be so kind to define your criteria for things that
> > "need fixing" so we could see if can reach some sort of an agreement on
> > this. My list is roughly as follows:
> >
> > - Erroneous use of kernel API
> > - Bad coding style
> > - Layering violations
> > - Duplicate code
> > - Hard to read code

> The reason people post their patches for review is to get good feedback
> on them. The problems you list above are mostly nitpicks. They must be
> fixed before inclusion of the patch, but only make sense to start fixing
> once the patch does a reasonable change.

If the coding style is an obstacle to understanding, it has to be fixed if
there is going to be any kind of review. Besides, nitpicks being simple to
address, they could as well be fixed while at it. Perhaps that way the
author learns to do it right, and less nitpicks are left in later additions
and fixes.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/