That looks fine - thanks Lars! On Mon, Jun 11, 2018 at 7:48 PM, Lars Volker <l...@cloudera.com> wrote:
> It seems that Jenkins now posted the "Build started" message twice. I > removed one of them from the config. Tim, can you verify that I removed the > right one? > > https://jenkins.impala.io/job/gerrit-verify-dryrun/jobConfigHistory/ > showDiffFiles?timestamp1=2018-06-11_22-09-04×tamp2= > 2018-06-12_02-47-10 > > On Mon, Jun 11, 2018 at 3:09 PM, Tim Armstrong <tarmstr...@cloudera.com> > wrote: > > > Ok, I applied the changes. Let me know if you run into any issues. > > > > On Mon, Jun 11, 2018 at 3:05 PM, Sailesh Mukil <sail...@cloudera.com> > > wrote: > > > > > +1 > > > > > > On Mon, Jun 11, 2018 at 3:02 PM, Jim Apple <jbap...@cloudera.com> > wrote: > > > > > > > No objection from me. > > > > > > > > On Mon, Jun 11, 2018 at 12:06 PM, Tim Armstrong < > > tarmstr...@cloudera.com > > > > > > > > wrote: > > > > > > > > > > On nit: as GVD gets more complex, it becomes harder for new > people > > to > > > > > understand the messages and +Ns applied to their patches. That > > doesn't > > > > mean > > > > > we shouldn't do this, only that it's something to keep an eye on. > > > > > > > > > > I think this helps with that problem in net by removing the manual > > > rebase > > > > > step that people have to remember to do. > > > > > > > > > > > > > > > On Mon, Jun 11, 2018 at 12:04 PM, Tim Armstrong < > > > tarmstr...@cloudera.com > > > > > > > > > > wrote: > > > > > > > > > > > I've tried my job a few times and it's working as expected. Any > > > > > objections > > > > > > to me switching over gerrit-verify-dryrun to my approach? > > > > > > > > > > > > On Thu, Jun 7, 2018 at 2:42 PM, Tim Armstrong < > > > tarmstr...@cloudera.com > > > > > > > > > > > wrote: > > > > > > > > > > > >> Ok, I was able to put together a test job that does the > automatic > > > > rebase > > > > > >> and carries a +2 here: https://jenkins.impala.io/job/ > > > > > >> gerrit-verify-dryrun-tarmstrong/ > > > > > >> > > > > > >> The diff in job config required to get it to work is here: > > > > > >> https://jenkins.impala.io/job/gerrit-verify-dryrun-tarmstron > > > > > >> g/jobConfigHistory/showDiffFiles?timestamp1=2018-06-07_20- > > > > > >> 41-18×tamp2=2018-06-07_21-38-58 > > > > > >> > > > > > >> I tested by creating some private drafts, adding "Impala Public > > > > Jenkins" > > > > > >> as a reviewer and testing the various scenarios. > > > > > >> > > > > > >> On Thu, Jun 7, 2018 at 2:26 PM, Jim Apple <jbap...@cloudera.com > > > > > > wrote: > > > > > >> > > > > > >>> I agree with both of you. > > > > > >>> > > > > > >>> On nit: as GVD gets more complex, it becomes harder for new > > people > > > to > > > > > >>> understand the messages and +Ns applied to their patches. That > > > > doesn't > > > > > >>> mean > > > > > >>> we shouldn't do this, only that it's something to keep an eye > on. > > > > > >>> > > > > > >>> On Thu, Jun 7, 2018 at 1:28 PM, Philip Zeyliger < > > > phi...@cloudera.com > > > > > > > > > > >>> wrote: > > > > > >>> > > > > > >>> > Seems fine, especially since we do the rebase as our > submission > > > > > >>> strategy > > > > > >>> > anyway, so we're already accepting/testing something that's > > > likely > > > > to > > > > > >>> get > > > > > >>> > rebased, and we may as well minimize that window. > > > > > >>> > > > > > > >>> > I'd be in favor of the bot also carrying the votes. > > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > > >>> > On Thu, Jun 7, 2018 at 1:24 PM, Tim Armstrong < > > > > > tarmstr...@cloudera.com > > > > > >>> > > > > > > >>> > wrote: > > > > > >>> > > > > > > >>> > > One annoyance with our precommit job is the requirement to > > > > manually > > > > > >>> > rebase > > > > > >>> > > the change before starting the merge. Failure to do so > either > > > > leads > > > > > >>> to > > > > > >>> > > false positives or false negatives - builds that failed > > because > > > > > they > > > > > >>> were > > > > > >>> > > missing a flaky/broken test fix and builds that succeeded > > > despite > > > > > >>> > > interacting badly with a previous fix. > > > > > >>> > > > > > > > >>> > > What do people think about modifying gerrit-verify-dryrun > to > > > > > >>> > automatically > > > > > >>> > > rebase the patch (by the programmatic equivalent of hitting > > the > > > > > >>> "Rebase" > > > > > >>> > > button) at the start of the job? The patch author would > still > > > > have > > > > > to > > > > > >>> > carry > > > > > >>> > > the +2 but this might make our lives a bit easier. > > > > > >>> > > > > > > > >>> > > - Tim > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > > > >
