[Koha-devel] Signing-off a patch for a customer

[Ian Walls]

To me, the role of QA is to be highly conservative.  The QA team needs to
look at a piece of incoming code, and not only judge it on how well it does
it's intended purpose, but how it affects all the other code and workflows
that surround it.  The folks creating and signing off on code are often
looking to answer the question "does it work?".  I approach the QA process
asking the question "what does it break?".

Often times, the answer is "nothing", and we get a great new feature in our
codebase. But sometimes the patch changes a core function or variable
declaration in a way that isn't spotted, and only applicable on certain use
cases.  Or a new dependency is added that conflicts with something
existing.  Or a security hole is introduced under some conditions.  A
person asking "does this work?" isn't necessarily going to spot these
things in their testing; our code is very complex.  I'm sure we can all
recall cases where piece of code was committed to do one thing, and then
required a followup because it broke something else under specific
circumstances.

I strongly believe that having a 'neutral party' to do the QA work is
essential to keeping our codebase strong and healthy.  We need the fresh
set of eyes, the different perspective, the alternate use case.  We need
someone asking "what does it break?", and I don't think the folks who've
been asking the question "does it work?" are the best suited to that task.

That said, if ANYONE sees a problem with a patch, I would encourage them to
note it on the bug report.  If s/he is confident this is a serious problem,
or a violation of well-documented coding practices, then please feel free
to set "Failed QA".  That status is open to everyone to set, because, as
stated earlier, QA is a highly conservative process, and I'd prefer a patch
have to wait a little longer to be re-evaluated than to have a new bug
introduced that could have been prevented with a word.

That's my perspective on the matter.  I'd also very much like to see it
made easier for libraries to provide their sign-off on code, as they're
often some of the best to evaluate "does it work?".


-Ian


On Tue, May 29, 2012 at 8:05 AM, Paul Poulain <paul.poulain at biblibre.com>wrote:

> Le 29/05/2012 13:50, Marcel de Rooy a écrit :
> > I agree with most responses in this thread: If a company makes a patch,
> a customer of that company signs off,
> > it should not be QAed by that company, but by a "neutral" party.
> > The QAer should even be allowed to ask for a second outside signoff if
> he feels the patch needs that additional proof.
>
> What's the QA done for ? it's looking at the quality of the code. So, a
> QAer can request a 2nd signoff, but only if he feel that the code is
> missing a case (like "I feel this code will work for UNIMARC, could
> someone check for MARC21" ? or "work for sysprefX = OFF, must be checked
> for sysprefX = ON as well". I feel that should be an uncommon case.
> The RM has a more global responsibility to ensure the global consistency
> of the soft.
>
> > As Paul mentioned earlier (he does QA but does not set the status):
> > In order to prevent the appearance of pushing the process,
> > you could even ask if it would be wiser to refrain from such QA
> comments. (Or just mail them to the author.)
> I'm not sure I understand ?
> Do you mean I should not QA if I can't set the status ? Sound a very bad
> idea : if I see something wrong, like an unconditionnal warn, a missing
> use Modern::Perl, it must be said publicly, to avoid having another QAer
> requesting this problem to be fixed
>
> Reminder (or information in case you don't know) = I usually QA & push
> patches ordered by last modification date, ASC (So the oldest 1st). If I
> see something wrong while I'm reviewing, I don't understand why I should
> stay silent !
>
> HTH
> --
> Paul POULAIN
> http://www.biblibre.com
> Expert en Logiciels Libres pour l'info-doc
> Tel : (33) 4 91 81 35 08
> _______________________________________________
> Koha-devel mailing list
> Koha-devel at lists.koha-community.org
> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
> website : http://www.koha-community.org/
> git : http://git.koha-community.org/
> bugs : http://bugs.koha-community.org/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/koha-devel/attachments/20120529/3b1a7d68/attachment.htm>