Hi,
bertagaz wrote (24 Feb 2015 14:12:14 GMT) :
> On Fri, Feb 20, 2015 at 11:56:02AM +0100, intrigeri wrote:
>> It currently reads:
>>
>> For base branches:
>>
>> * When failing after a git push, notify the last commit author.
>> * When failing after the daily trigger, notify the RM.
>>
>> I say let's just always the RM to start with.
> I'm not sure about that still: if we proceed this way, we're not
> implementing the reviewer scenario, because the RM will be notified when a
> branch is merged and breaks the build, and not the one who did this merge.
It seems there's a serious misunderstanding here.
The way I understand the reviewer scenario has no specific provision
regarding whatever should happen *after* branch F was *really* merged
into branch B by the reviewer.
Reviewing happens *before* merging, and part of the review work is
ensuring that the resulting branch B (after branch F is merged into
it) will still build fine. Of course reviewers must not do this part
of their job *after* merging and pushing to the official repo, so this
is only meaningful if done *before*.
IMO the reviewer scenario we have is meant to address this specific
problem, to make the reviewers' work easier (hence the "I might want
to download the resulting ISO" part for example, which only makes
sense before merging for real), thanks to Jenkins and to your work.
The way I understand it (and the way I think it should be), in:
As a reviewer
When I'm asked to review branch F into branch B
Then I need to know if branch F builds fine
once merged into branch B (fresh results!)
And I should be notified of the results
And if the build succeeded
I might want to download the resulting ISO
I might want to get the pkg list
I want the Redmine ticket to be notified (optional)
Otherwise if it fails the developer who proposed the merge should be notified
And the developer *needs* to see the build logs
And the ticket should be reassigned to the branch submitter
And QA check should be set to "Dev needed"
... "once merged into branch B" is a local merge, done in Jenkins
only, and *not* pushed to the official repo.
Otherwise the remaining steps don't make any sense to me: if the build
fails, we don't want to say "oh too bad, we have merged branch F
already, so our build is broken, but let's mark the ticket as dev
needed and reassign to the developer". Instead, we want to simply
*not* merge this branch yet, and then mark the ticket as dev needed
and reassign to the developer.
And, if the reviewer mistakenly merges something that breaks the
build, then of course I would consider it's their responsibility for
fixing it, ideally... but we can't count on reviewers to be around,
notice the failure and fix it in a timely manner: as you know,
reviewers come and go, and sometimes disappear for a week or more.
The only person we have on formal duty for such things, who's taking
care of the state of our base branches on which the next release(s)
will be based, is the RM. So IMO the only reliable solution is to
notify the RM whenever this situation happens (which should be pretty
rare anyway :)
Thoughts?
Cheers,
--
intrigeri
