quality control

quality control

[Hans Reiser]

Dearest HCH,

Please consider adhering to a quality control process.  No patch comes
out of Namesys without a second person testing it (that includes
compiling it).  Users should not be burdened with code that has not been
tested by a second person.  Everyone makes mistakes of this kind, the
difference is that some persons use a quality control process to avoid
burdening more than one other person with them.

Hans

Jeff Mahoney wrote:

> Hans Reiser wrote:
>
> >http://bugzilla.kernel.org/show_bug.cgi?id=6016
>
> >           Summary: reiserfs doesn't build with REISERFS_FS_POSIX_ACL=n
> >    Kernel Version: v2.6.16-rc2-g5b7b644
> >            Status: NEW
> >          Severity: normal
> >             Owner: reiserfs-dev@namesys.com
> >         Submitter: kernel-bugzilla@luksan.cjb.net
>
>
> >Most recent kernel where this bug did not occur: 2.6.16-rc1
> >Distribution: Gentoo
> >Hardware Environment: amd64
> >Problem Description:
>
> >reiserfs doesn't build with REISERFS_FS_POSIX_ACL=n
>
> >Steps to reproduce:
> >$ make
> >  CHK     include/linux/version.h
> >  SPLIT   include/linux/autoconf.h -> include/config/*
> >  CHK     include/linux/compile.h
> >  CHK     usr/initramfs_list
> >  GZIP    kernel/config_data.gz
> >  IKCFG   kernel/config_data.h
> >  CC      kernel/configs.o
> >  LD      kernel/built-in.o
> >  CC      fs/reiserfs/xattr.o
> >fs/reiserfs/xattr.c: In function `reiserfs_check_acl':
> >fs/reiserfs/xattr.c:1330: error: called object is not a function
> >make[2]: *** [fs/reiserfs/xattr.o] Error 1
> >make[1]: *** [fs/reiserfs] Error 2
> >make: *** [fs] Error 2
>
> >Reverting ec191574b9c3cb7bfb95e4f803b63f7c8dc52690
> >---
> >[PATCH] reiserfs: use generic_permission
>
> >Use the generic_permission code with a proper wrapper and callback
> instead
> >of having a local copy.
> >---
> >fixes the problem, but causes some warnings about unused symbols.
>
>
> This was a patch from hch, not me. There's already a patch in -mm to
> fix it.
>
> -Jeff
>
> --
> Jeff Mahoney
> SUSE Labs

Re: quality control

[Kyle Moffett]

On Feb 05, 2006, at 22:15, Hans Reiser wrote:
> Jeff Mahoney wrote:
>> Hans Reiser wrote:
>>> http://bugzilla.kernel.org/show_bug.cgi?id=6016
>>>           Summary: reiserfs doesn't build with  
>>> REISERFS_FS_POSIX_ACL=n
>>>    Kernel Version: v2.6.16-rc2-g5b7b644
>>
>> This was a patch from hch, not me. There's already a patch in -mm to
>> fix it.
>
> Please consider adhering to a quality control process.

It's a GIT version of an RC patch for grief's sake!  You don't  
seriously expect people to quadruple-check every trivial patch that  
goes into Linus GIT tree before sending it, do you?  The whole point  
of the RC is to indicate that only smaller patches should be applied  
(and this one was for the most part) so that we can do some kind of  
global-kernel QC.

> Everyone makes mistakes of this kind, the difference is that some  
> persons use a quality control process to avoid burdening more than  
> one other person with them.

Precisely!  The guy who reported the bug is the one person who was  
burdened with it.  It will get fixed in the GIT tree, and only  
somebody who happened to test the dev tree between the two patches  
with that particular .config will have noticed.

BTW: Nice way to CC a private thread to a public list without the  
consent of all parties.  You also made the grievous errors of (A) top- 
posting, (B) fullquoting without snipping irrelevant material, and  
(C) sending flamebait to the list (which I am so stupidly responding  
to).

Cheers,
Kyle Moffett

--
Debugging is twice as hard as writing the code in the first place.   
Therefore, if you write the code as cleverly as possible, you are, by  
definition, not smart enough to debug it.
   -- Brian Kernighan

Re: quality control

[Jens Axboe]

On Sun, Feb 05 2006, Kyle Moffett wrote:
> On Feb 05, 2006, at 22:15, Hans Reiser wrote:
> >Jeff Mahoney wrote:
> >>Hans Reiser wrote:
> >>>http://bugzilla.kernel.org/show_bug.cgi?id=6016
> >>>          Summary: reiserfs doesn't build with  
> >>>REISERFS_FS_POSIX_ACL=n
> >>>   Kernel Version: v2.6.16-rc2-g5b7b644
> >>
> >>This was a patch from hch, not me. There's already a patch in -mm to
> >>fix it.
> >
> >Please consider adhering to a quality control process.
> 
> It's a GIT version of an RC patch for grief's sake!  You don't  
> seriously expect people to quadruple-check every trivial patch that  
> goes into Linus GIT tree before sending it, do you?  The whole point  
> of the RC is to indicate that only smaller patches should be applied  
> (and this one was for the most part) so that we can do some kind of  
> global-kernel QC.

Eh, but you are expected to do that. If everybody sent in half-assed not
tested patches "just because" it's a pre-rc, things would look bad.
Compile testing is the least time consuming and easiest thing to to
test, so you should at least do that. If nobody did that, no one would
get snapshots tested because they would never compile for anyone.

For developers it's equally annoying. I typically update my tree every
morning and run with that for the various stuff I'm working on, and it
is really annoying to have to spent time on hunting down and fixing
compile errors.

This mail is about the mentality displayed, not the specific change that
spawned it.

-- 
Jens Axboe

Re: quality control

[Martin Mares]

> BTW: Nice way to CC a private thread to a public list without the  
> consent of all parties.  You also made the grievous errors of (A) top- 
> posting, (B) fullquoting without snipping irrelevant material, and  
> (C) sending flamebait to the list (which I am so stupidly responding  
> to).

... which is a nice co-incidence in a thread on quality control :)))

					Martin

Re: quality control

[Andi Kleen]

Kyle Moffett <mrmacman_g4@mac.com> writes:
> 
> It's a GIT version of an RC patch for grief's sake!  You don't
> seriously expect people to quadruple-check every trivial patch that
> goes into Linus GIT tree before sending it, do you? 

No quadruple check, but every patch going to Linus should get at least
some basic testing and it's definitely suppose to compile at least
in one .config combination.

> The whole point
> of the RC is to indicate that only smaller patches should be applied
> (and this one was for the most part) so that we can do some kind of
> global-kernel QC.

global kernel QA doesn't replace individual patch QA. Black box 
testing is no replacement for targetted tests.

-Andi

Re: quality control

[Kyle Moffett]

On Feb 06, 2006, at 06:09, Andi Kleen wrote:
> Kyle Moffett <mrmacman_g4@mac.com> writes:
>> It's a GIT version of an RC patch for grief's sake!  You don't  
>> seriously expect people to quadruple-check every trivial patch  
>> that goes into Linus GIT tree before sending it, do you?
>
> No quadruple check, but every patch going to Linus should get at  
> least some basic testing and it's definitely suppose to compile at  
> least in one .config combination.

Well, yes, and it did.  The problem was that if you turned off ACLs,  
it didn't work; only one or two variants of about 6 or 8 ways to  
configure reiserfs stopped working.  Given that, I can't see how Hans  
is complaining about lack of QC.  Nobody is going to test patches  
against every possible kernel configuration; that's why we do an RC,  
so that we can get a lot of different configs tested.

Cheers,
Kyle Moffett

--
I didn't say it would work as a defense, just that they can spin that  
out for years in court if it came to it.
   -- Rob Landley

Re: quality control

[Jens Axboe]

On Mon, Feb 06 2006, Kyle Moffett wrote:
> On Feb 06, 2006, at 06:09, Andi Kleen wrote:
> >Kyle Moffett <mrmacman_g4@mac.com> writes:
> >>It's a GIT version of an RC patch for grief's sake!  You don't  
> >>seriously expect people to quadruple-check every trivial patch  
> >>that goes into Linus GIT tree before sending it, do you?
> >
> >No quadruple check, but every patch going to Linus should get at  
> >least some basic testing and it's definitely suppose to compile at  
> >least in one .config combination.
> 
> Well, yes, and it did.  The problem was that if you turned off ACLs,  
> it didn't work; only one or two variants of about 6 or 8 ways to  
> configure reiserfs stopped working.  Given that, I can't see how Hans  

Look, it's really simple: lets say I make a change that has to do with
PM, you do a quick compile test with and _without_ PM just to check you
didn't screw anything up with that change. You change reiserfs acl
stuff, you do a quick compile test with and without that configured.

It's a pretty standard procedure, and contrary to what you think, it
_is_ required before submitting a patch. No one is asking anyone to
check all possible configure options, but the interesting data set is
typically extremely easy to guess looking at a change.

-- 
Jens Axboe

Re: quality control

[Andrew Morton]

Jens Axboe <axboe@suse.de> wrote:
>
> Look, it's really simple: lets say I make a change that has to do with
> PM, you do a quick compile test with and _without_ PM just to check you
> didn't screw anything up with that change. You change reiserfs acl
> stuff, you do a quick compile test with and without that configured.
> 
> It's a pretty standard procedure, and contrary to what you think, it
> _is_ required before submitting a patch. No one is asking anyone to
> check all possible configure options, but the interesting data set is
> typically extremely easy to guess looking at a change.

<rofl>

bix:/usr/src/op> find patches -name '*build-fix*' | wc -l
    533

bix:/usr/src/op> find patches -name '*fix.patch' | wc -l
   5109

A lot of people don't make the slightest effort.  But it's not a big
problem, really.  Silly build errors are reported early and are almost
always trivial to fix.  The major drawback is that they can wreck a -mm
release for many testers.

Re: quality control

[Jens Axboe]

On Tue, Feb 07 2006, Andrew Morton wrote:
> Jens Axboe <axboe@suse.de> wrote:
> >
> > Look, it's really simple: lets say I make a change that has to do with
> > PM, you do a quick compile test with and _without_ PM just to check you
> > didn't screw anything up with that change. You change reiserfs acl
> > stuff, you do a quick compile test with and without that configured.
> > 
> > It's a pretty standard procedure, and contrary to what you think, it
> > _is_ required before submitting a patch. No one is asking anyone to
> > check all possible configure options, but the interesting data set is
> > typically extremely easy to guess looking at a change.
> 
> <rofl>
> 
> bix:/usr/src/op> find patches -name '*build-fix*' | wc -l
>     533
> 
> bix:/usr/src/op> find patches -name '*fix.patch' | wc -l
>    5109
> 
> A lot of people don't make the slightest effort.  But it's not a big
> problem, really.  Silly build errors are reported early and are almost
> always trivial to fix.  The major drawback is that they can wreck a -mm
> release for many testers.

That's precisely the problem, it may be really simple to fix but often
will stop people from testing.

Your fix count probably isn't totally accurate either, I bet a lot of
these are fixups due to conflicts with other patches. I'm talking about
the fact that someone sends Linus a patch which doesn't compile for the
case you could (and should) have trivially checked. A little edumacation
never hurt :-)

-- 
Jens Axboe

Re: quality control

[Olaf Hering]

 On Mon, Feb 06, Andi Kleen wrote:

> Kyle Moffett <mrmacman_g4@mac.com> writes:
> > 
> > It's a GIT version of an RC patch for grief's sake!  You don't
> > seriously expect people to quadruple-check every trivial patch that
> > goes into Linus GIT tree before sending it, do you? 
> 
> No quadruple check, but every patch going to Linus should get at least
> some basic testing and it's definitely suppose to compile at least
> in one .config combination.

Right. We have now git-bisect, and it helped me to nail down a few bugs.
Just now I track down some scsi or whatever breakage in -rc1. And guess
what, not a single compile error so far, with a full featured config!
So you guys better send tested patches, via akpm, to keep Linus tree in
a reasonable shape.

-- 
short story of a lazy sysadmin:
 alias appserv=wotan