On 09/01/2015 12:23 PM, intrigeri wrote:
> bertagaz wrote (26 Aug 2015 17:52:26 GMT) :
>> On Wed, Aug 26, 2015 at 03:38:19PM +0200, anonym wrote:
>>> The current proposal seems to be to only start the automated test run of
>>> a feature branch when it is marked "Ready for QA". This has overloaded
>>> the meaning of this status so it no longer alone means that the branch
>>> is ready for review'n'merge; the reviewer also has to wait until the
>>> automated tester posts the result to the ticket.
>>>
>>> We could get rid of this ambiguity by splitting "Ready for QA" into
>>> "Ready for (automated) testing" (RFT) and "Ready for review" (RFR). Example:
>>>
>>> Let's say I have created a new feature branch and think I'm done. I
>>> assign it to intrigeri (who I want to review it in the end) and mark it
>>> RFT. And automated build is triggered, and then a full run of the test
>>> suite. Let's say the test suite fails. Then it gets reassigned to me and
>>> marked "Dev needed". I fix the issue (which probably was easier to spot
>>> for me than it would be for intrigeri, since I did the implementation)
>>> assign it to intrigeri and mark it RFT again. This time the test suite
>>> passes, and then the ticket is marked RFR automatically. Also, if I run
>>> the test suite myself and see it pass, then I can just mark it RFR directly.
>
>> I've think about this issue too. My own conclusion was to add a new
>> Custom fiels in redmine, that Jenkins would use, so something similar to
>> yours. The dev mark the ticket as ReadyForQA, then Jenkins run the test
>> suite on it and send its report modifying that field accordingly.
>
> The way it's conceptualized in Gerrit (and presumably in other similar
> tools) is that Jenkins is "just another reviewer", that can vote +1
> or -1 on a merge request. I like this logic. To translate it into
> Redmine, RfQA means that all reviewers (humans and robots) can start
> looking at the branch, and Pass means that all reviewers are happy
> with it.
Conceptually, I like this.
> I think it would be overly complicated to encode individual
> reviews (e.g. the one done by Jenkins) in the QA Check field, and
> conceptually I prefer to keep QA Check a bit more high level and not
> give it a finer granularity.
I certainly agree that my proposal complicates things.
> So, adding dedicated custom fields seems to be the best option to
> encode individual review results: "Jenkins OK" would be unset
> initially, and set to true (= +1) upon successful testing. Upon failed
> testing:
>
> * "QA Check" would be set back to "Dev Needed";
> * a negative vote from Jenkins is a blocker, so given QA Check has
> been reset already, I'm not sure it's useful to also set "Jenkins
> OK" to "false" (which we would have to revert after pushing a fix
> and setting QA Check = RfQA).
>
> ... and "Human reviewer OK" would work just the same.
Since pushing stuff into the branch after this field has been set to
true invalidates the Jenkins' test suite run, would Jenkins monitor for
this and unset the field, or is it up to the committer to unset it? I
realize this is not a problem unique to this solution. Any way, doing
this manually gets hairy since we don't necessarily know which commit
Jenkins has tested. I suppose it would help if Jenkins also posted a
message about what commit it has successfully tested. Or maybe the field
we want to add instead could contain the commit?
> But this doesn't address the problem anonym pointed to initially, that
> is "the reviewer also has to wait until the automated tester posts the
> result to the ticket".
And the corollary is that it neither solves the problem: reviewers may
waste time reviewing a branch that breaks an automated test. That's the
important part, IMHO.
> One possible solution would be to assign RfQA
> tickets to Jenkins initially, and once Jenkins has voted +1 (and set
> "Jenkins OK" to true), it could also unassign the ticket from itself,
> and then human reviewers can look into it. Jenkins would still run
> automated tests on branches regardless of their ticket's assignee, as
> specified elsewhere, but at least this would make it clear to human
> reviewers when it's time for them to start reviewing stuff.
Sure. I guess Jenkins would only assign itself, and not assign the
intended human reviewer (since that info isn't available). That seems a
bit awkward to me, to (as the implementer) have to return to the tickets
when Jenkins is done, and then assign the human reviewer.
Cheers!
