I have 2 questions for the QA team.
1/ About bug 8687, a proposed patch (http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12713&action=diff) removes perlcritic warnings. This way of doing is particularly brutal, it completely removes the code :)
So we have 2 solutions :
- To propose a patch to replace ALL
eval { require "$FindBin::Bin/../kohalib.pl" };
with
my $lib = "$FindBin::Bin/../kohalib.pl"
eval { require $lib};
in the misc scripts.
Like this, perlcritic is happy...
- We add a prerequisite for the misc scripts : the PERL5LIB environment variable has to be set with the koha path and we delete these lines.
I'd prefer we fix our perlcritic configuration. Quoted requires have legitimate uses. Barring that, I think I prefer the first option, provided it is *really* equivalent. On the other hand, if we're requiring the user to have KOHA_CONF set, perhaps there's no reason that PERL5LIB shouldn't be required as well...
Okay, I guess I don't actually have an opinion on this, other than to note that any perlcritic fixes need to be thoroughly tested, especially those involving eval {}s. Inductive proofs that a given change does not break any functionality would be particularly welcome.
2/ Recently I suggested a followup (to fix QA issues) for a bug (bug 7067) and I switched the status to 'passed qa'. I thought it was easier to let the RM review a simple patch instead of asking for a new sign off and another QA. I understand that we don't want to have patches without review and test. But in this case (and in most cases when I suggest a new patch while passing QA) it is a simple patch and starting again a new iteration (SO + QA) burn off energy and/or could block a patch for several weeks in this status.
Maybe this point is a non-problem and will be fixed with new status :)
Ordinarily I'd take a follow-up patch that fixes QA problems without additional signoffs. In this case I asked for additional signoffs because there were so many comments on the bug I was concerned I might miss something when testing. There had been 103 comments when I sat down to review the patches, which is a lot of scope for me to miss a previously-identified edge case or regression.
Regards,
Jared