On 26 November 2012 21:20, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:

Jared,

Could have missed something, but was this decided recently?

The additional signoff will mean more workload for the QA team, since it requires additional testing. Which is fine of course in theory, but may lead to a larger queue of patches waiting for QA, etc.

It shouldn't change much in the way of workflow, surely everyone is applying the patches they QA? Its a simple step with git-bz to signoff and attach that patch. Can't really pass QA on a patch that doesn't even apply, right? Chris

On 26 November 2012 21:25, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:

Testing is more than checking if it applies :)

Of course it is, but you can hardly QA something if you haven't even applied it. My point is, applying a patch then doing your QA then signing that patch off ... the only change is adding the sign off. All Jared is asking is, that if you mark a patch passed-qa you add a sign off to it. Chris

Hi Marcel and Chris, I think what you are discussing here is basically the definition of 'Passed QA'. I think setting this status means, that I am sure that a patch works and does not cause any regressions. I would like QA team members only to set this status, if they are sure. Else what we do is pass this task on to our RM, who is only one person and has to deal with a lot of more work than any individual member of the QA team. So the next question is, what can we do to be sure something works? - make sure the patch applies - make sure it passes all the tests - step through the coding guidelines and review the code - do functionality testing - add our sign-off If the documentation on the bug is not enough, to make it possible to test or understand what a patch is supposed to do - we should ask for more information. Everyone misses something sometimes, but that is why we have different people looking at patches. Even with a solid sign-off, there might still be something to catch and there is nothing wrong with missing something or with catching it. :) Katrin

-----Original Message----- From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel- bounces@lists.koha-community.org] On Behalf Of Marcel de Rooy Sent: Monday, November 26, 2012 9:42 AM To: Chris Cormack; koha-devel@lists.koha-community.org Subject: Re: [Koha-devel] [QA] QAing and signing off

For me it feels more work. If I add a signoff, I must have tested the patch more extensively. But if I do not need to add signoff and I can rely on a solid signoff already, the QA process will be faster. Note that I do not oppose this step, I am just warning that it takes more time. I do not favor only adding a signoff line without more testing. This would create a "false feeling of security".

Marcel

-----Oorspronkelijk bericht----- Van: Chris Cormack [mailto:chris@bigballofwax.co.nz] Verzonden: maandag 26 november 2012 9:28 Aan: Marcel de Rooy CC: Jared Camins-Esakov; koha-devel@lists.koha-community.org Onderwerp: Re: [Koha-devel] [QA] QAing and signing off

On 26 November 2012 21:25, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:

...

Testing is more than checking if it applies :)

Of course it is, but you can hardly QA something if you haven't even applied it. My point is, applying a patch then doing your QA then signing that patch off ... the only change is adding the sign off.

All Jared is asking is, that if you mark a patch passed-qa you add a sign off to it.

Chris _______________________________________________ Koha-devel mailing list Koha-devel@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/

On 26 November 2012 22:27, Paul Poulain <paul.poulain@biblibre.com> wrote:

Le 26/11/2012 10:02, Fischer, Katrin a écrit :

...

So the next question is, what can we do to be sure something works?

- make sure the patch applies - make sure it passes all the tests - step through the coding guidelines and review the code - do functionality testing If we require that, it mean that the QA team should have to repeat the signoff-er responsibility.

Until now, we said the QA goal was to be concentrated on code quality, testing the feature was made 'somewhere else'. (that's why I also think we could have QA before or after signoff, that's 2 independant things. But I don't see how to achieve that with bugzilla, so I never suggested the idea)

Hmm I always thought the first sign off was just testing the patch works, QA would test for code quality, regressions, etc. At least that's what I expected when I was doing RM. It sounds like it changed a bit while Paul was RM, and that is fine after all it was his responsibility to decide how he wanted to run his releases. We probably need to await Jared awaking, and clarifying exactly what he expects from QA for the 3.12 release, since that's what we elected him for :) Chris

On 26 November 2012 23:13, Paul Poulain <paul.poulain@biblibre.com> wrote:

Le 26/11/2012 10:34, Chris Cormack a écrit :

...

...

Until now, we said the QA goal was to be concentrated on code quality, testing the feature was made 'somewhere else'. (that's why I also think we could have QA before or after signoff, that's 2 independant things. But I don't see how to achieve that with bugzilla, so I never suggested the idea)

Hmm I always thought the first sign off was just testing the patch works, QA would test for code quality, regressions, etc. That's what I wrote -in better english probably ;-) -, or am I silly today ?

Not sure, in testing for regressions, you have to apply the patch, test it, and test it breaks nothing else, in effect testing more than the first signoff, not testing less. I thought you were saying that QA just focus on code quality, not whether it breaks something else or not. Chris

On 2012-11-27, at 2:52 AM, Jared Camins-Esakov wrote:

From my point of view, the reason we elected the six people we did for the QA team is that we know that those six people understand the big picture of Koha better than other developers, and would be able to identify places where regressions are likely. Code quality is important, but less important than the end user experience, which would be much more marred by the presence of a regression than the presence of a backtick. So it is my expectation that when QAing patches our QA team is doing at least some testing (for the most part I believe this has been done throughout the 3.12 cycle; Marcel, an example of what I'm looking for can be found on http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8206 where you spotted a number of problems in the course of QA). So I don't see that this requirement adds a significant burden on the QA team. After all, you're already applying the patch.

On the other hand, the presence of the Signed-off-by line from the QA team *greatly* reduces the burden on me, and since I have had to push 114 patches (plus merges) *by myself* since October 30 (and reviewed a fair number more), I think the tradeoff is worth it. As an example of what I'm talking about, there was one bug where the author repeatedly marked it "Passed QA" himself, without anyone else looking at the code or testing it. In order to figure out what happened, I had to wade through 67 comments on the bug and several pages worth of history, just to find that no, I hadn't missed the QA team's approval, in fact the patches never passed QA. There can be no question that I am the bottleneck for code getting into Koha, and the day that bug wasted was taken from my review of bug 7067, which probably would have applied had I been able to use that time to review it.

Regards, Jared

anything the QA team can do to make the Jared's RM work easier, i am happy to do so... i don't mind taking a little extra time to 'sign-off' patches when i QA them - to help Jared save a lot of time +1 from me, for the QA-team adding additional 'sign-off' on patches, during QA process

+1 for QA signoffs; it seems a trivial addition. On Mon, Nov 26, 2012 at 8:52 AM, Jared Camins-Esakov < jcamins@cpbibliography.com> wrote:

...

Until now, we said the QA goal was to be concentrated on code quality,

...

testing the feature was made 'somewhere else'. (that's why I also think we could have QA before or after signoff, that's 2 independant things. But I don't see how to achieve that with bugzilla, so I never suggested the idea)

Hmm I always thought the first sign off was just testing the patch works, QA would test for code quality, regressions, etc. At least that's what I expected when I was doing RM. It sounds like it changed a bit while Paul was RM, and that is fine after all it was his responsibility to decide how he wanted to run his releases. We probably need to await Jared awaking, and clarifying exactly what he expects from QA for the 3.12 release, since that's what we elected him for :)

From my point of view, the reason we elected the six people we did for the QA team is that we know that those six people understand the big picture of Koha better than other developers, and would be able to identify places where regressions are likely. Code quality is important, but less important than the end user experience, which would be much more marred by the presence of a regression than the presence of a backtick. So it is my expectation that when QAing patches our QA team is doing at least some testing (for the most part I believe this has been done throughout the 3.12 cycle; Marcel, an example of what I'm looking for can be found on http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8206 where you spotted a number of problems in the course of QA). So I don't see that this requirement adds a significant burden on the QA team. After all, you're already applying the patch. On the other hand, the presence of the Signed-off-by line from the QA team *greatly* reduces the burden on me, and since I have had to push 114 patches (plus merges) *by myself* since October 30 (and reviewed a fair number more), I think the tradeoff is worth it. As an example of what I'm talking about, there was one bug where the author repeatedly marked it "Passed QA" himself, without anyone else looking at the code or testing it. In order to figure out what happened, I had to wade through 67 comments on the bug and several pages worth of history, just to find that no, I hadn't missed the QA team's approval, in fact the patches never passed QA. There can be no question that I am the bottleneck for code getting into Koha, and the day that bug wasted was taken from my review of bug 7067, which probably would have applied had I been able to use that time to review it.

Regards, Jared

-- Jared Camins-Esakov Bibliographer, C & P Bibliography Services, LLC (phone) +1 (917) 727-3445 (e-mail) jcamins@cpbibliography.com (web) http://www.cpbibliography.com/

_______________________________________________ Koha-devel mailing list Koha-devel@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/

Hi Marcel, My comment was directed at the additional burden presented by adding a '-s' and not a reflection of my opinion or view of the importances of the duty of QA. In reality, I'm surprised that this request is even an issue. All QA members should already have been accessing the quality of patches at least to the same degree as those doing generic sign-offs and really to a greater degree. So as I say, the burden of '-s' is, indeed, trivial. Kind Regards, Chris On Wed, Nov 28, 2012 at 9:27 AM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl>wrote:

The discussion was certainly not meant to be trivial :)

And the sign-off should not be either.. ------------------------------ *Van:* koha-devel-bounces@lists.koha-community.org [ koha-devel-bounces@lists.koha-community.org] namens Chris Nighswonger [ cnighswonger@foundations.edu] *Verzonden:* woensdag 28 november 2012 15:13 *To:* Jared Camins-Esakov *Cc:* koha-devel@lists.koha-community.org

*Onderwerp:* Re: [Koha-devel] [QA] QAing and signing off

+1 for QA signoffs; it seems a trivial addition.

Paul,

All QA members should already have been accessing the quality of

...

patches at least to the same degree as those doing generic sign-offs and really to a greater degree. So as I say, the burden of '-s' is, indeed, trivial. What is not-fun is that for string or really trivial changes, the QA is also trivial, and applying/signing/attaching the patch is "costly".

Jared, would you be OK to discard this "QA need a signature" for this kind of patch ? (reminder: now we have the patch complexity)

I'm not sure what the compelling argument for this would be. When you can QA a string change by sight off of bugzilla, you can use the following one-liner: `git-bz -s apply XXXX && git-bz attach -e HEAD` Indeed, you could even create a git alias, something like: qastring = !sh -c 'git checkout -b stringy origin/master && git-bz -s apply $0 && git-bz attach -e HEAD $0 && git branch -D stringy' That's what I'd do. Well, actually, I'd also have it run the template tests: qastring = !sh -c 'git checkout -b stringy origin/master && git-bz -s apply $0 && prove xt/tt_valid.t xt/author/translatable-templates.t xt/author/valid-templates.t && git-bz attach -e HEAD $0 && git branch -D stringy' Bingo! Look at a string patch, and if it looks good you run `git qastring XXXX` and as part of the upload you can add your QA comments and update the status. No more work than updating the bug manually, and probably less. Regards, Jared -- Jared Camins-Esakov Bibliographer, C & P Bibliography Services, LLC (phone) +1 (917) 727-3445 (e-mail) jcamins@cpbibliography.com (web) http://www.cpbibliography.com/

Le 26/11/2012 09:20, Marcel de Rooy a écrit :

Jared, The additional signoff will mean more workload for the QA team, since it requires additional testing. Which is fine of course in theory, but may lead to a larger queue of patches waiting for QA, etc.

I agree that it's something we should do, and I already started on a few patches I QAed before the 3.10, even if they were patches for 3.12 With git-bz, it just takes a few seconds, not a big deal. I just want to add/suggest that this requirement is not applied for string patches that are obviously OK and don't need to be tested. (that remind me I must do some changes to bugzilla , to add "ENH: New feature", and patch complexity field. Will do that now ;-) ) -- Paul POULAIN - BibLibre http://www.biblibre.com Free & Open Source Softwares for libraries Koha, Drupal, Piwik, Jasper