About merging patches without tests

David Caro dcaroest at redhat.com
Wed May 21 15:32:52 UTC 2014


On Wed 21 May 2014 04:28:10 PM CEST, Itamar Heim wrote:
> On 05/21/2014 05:25 PM, Eyal Edri wrote:
>>
>>
>> ----- Original Message -----
>>> From: "Itamar Heim" <iheim at redhat.com>
>>> To: "Sandro Bonazzola" <sbonazzo at redhat.com>, "David Caro"
>>> <dcaroest at redhat.com>
>>> Cc: "infra" <infra at ovirt.org>
>>> Sent: Friday, May 16, 2014 9:48:19 PM
>>> Subject: Re: About merging patches without tests
>>>
>>> On 05/16/2014 05:38 AM, Sandro Bonazzola wrote:
>>>> Il 16/05/2014 10:37, David Caro ha scritto:
>>>>> On Fri 16 May 2014 09:33:44 AM CEST, Sandro Bonazzola wrote:
>>>>>> Il 15/05/2014 18:46, David Caro ha scritto:
>>>>>>> Hi!
>>>>>>>
>>>>>>>   From time to time we have some patches that are merged to master
>>>>>>>   branches
>>>>>>> without having been tested mostly because the developer merges before
>>>>>>> having any
>>>>>>> response from the jenkins system.
>>>>>>>
>>>>>>> Merging one of those patches makes any following test run on that branch
>>>>>>> to
>>>>>>> fail, and creating a lot of noise and trouble around all patches and
>>>>>>> jobs.
>>>>>>>
>>>>>>> So I wanted to stat a little discussion to bring up ideas on how to
>>>>>>> prevent
>>>>>>> that. Some random thoughts:
>>>>>>>
>>>>>>> * -1 the patch at jenkins job start, and reset to 0 on success or infra
>>>>>>> failure,
>>>>>>> and keep -1 if jenkins failure
>>>>>>> * Only send a message to the patch with 'jenkins jobs started'
>>>>>>
>>>>>> I think that something like "jenkins jobs started, please don't merge
>>>>>> until they finish" should be enough.
>>>>>>
>>>>>>> * Setup zuul as gateway, and make it block the patches if they do not
>>>>>>> pass the tests
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Cheers!
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Infra mailing list
>>>>>>> Infra at ovirt.org
>>>>>>> http://lists.ovirt.org/mailman/listinfo/infra
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Usually the flow is reabase -> submit, that is done pretty easily from
>>>>> the ui, and it does not give too much time to check any comments (the
>>>>> rebase and submit buttons are on the top, and the comments show at the
>>>>> end).
>>>>> So doing that will not help on that case, as the developer would not
>>>>> see the comment before submitting. I think we should block, at least
>>>>> for a couple minutes, so he has to read the comments to be able to
>>>>> submit.
>>>>>
>>>>> We had that before also, adding a comment when the jobs start, but we
>>>>> removed it by developers request iirc
>>>>
>>>> Also recently you can just press submit and it automatically rebase before
>>>> merging.
>>>> So maybe yes, just a message isn't enough
>>>
>>> its not possible to wait for the job on a simple rebase usually - too
>>> many collisions possible.
>>
>> we're mostly talking about jobs that fail to compile, i think in those cases,
>> it worth to wait for jenkins job to finish.
>
> if the previous patch failed to compile - i agree.
> if a rebase of a patch that passed previously, waiting for another round of
> jobs isn't practical if other merge in the meantime. I'd say:
> - wait for it if last patch is several days old.
> - make sure to check the email from automation post the merge and revert/fix
> if it broke in a short loop.
>

Maybe we can retake the initiative of testing 'on demand', meaning, 
that the high load jobs only run after a maintainer +1 and a verified 
+1, that added to the extra flag might help ease the load and keep 
master stable:

Devel sends patches/rebase/whatever, verifies +1, then a maintainer 
reviews +1 -> jenkins job starts and adds the 'tested' flag when 
finished -> maintainer can submit

But e will need some control over the maintainers of each project (we 
have more or less that already as gerrit groups and permissions).

--
David Caro

Red Hat S.L.
Continuous Integration Engineer - EMEA ENG Virtualization R&D

Email: dcaro at redhat.com
Web: www.redhat.com
RHT Global #: 82-62605

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ovirt.org/pipermail/infra/attachments/20140521/31f0d872/attachment.sig>


More information about the Infra mailing list