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" <ih...@redhat.com> >>> To: "Sandro Bonazzola" <sbona...@redhat.com>, "David Caro" >>> <dcaro...@redhat.com> >>> Cc: "infra" <infra@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@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: dc...@redhat.com Web: www.redhat.com RHT Global #: 82-62605
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra