> 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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> >>> 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 <[email protected] >>> > >>> > 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 >>> > > >>> > >>> >> >> >
