Having submitted a patch to the ghprb-plugin repo before, I think that regretfully option (b) is probably the right decision here given that it's unlikely to get accepted, merged, released, and to have Infra update the plugin in under a week.
On Wed, Jun 13, 2018 at 10:42 PM Scott Wegner <sweg...@google.com> wrote: > Indeed, I was going to send out an email about pre-commit filtering, but > we've already found some kinks and may need to revert it. > > The change was submitted in PR#5611 [1] and enables Jenkins triggering to > only run pre-commits based on modified files. However, Udi noticed that > this also prevents manually running pre-commits on a PR with trigger > phrases when your PR changes don't match the pre-commit include path [2]. > This was blocking 2.5.0 release validation, so I have a PR out to revert > the change [3]. > > I did some investigation and this is a deficiency in the Jenkins plugin > used to trigger jobs on pull requests. I've filed a bug [4] and submitted a > PR [5], but there's no guarantee that it'll get accepted or when it will be > available. > > Question for others: we were hoping to enable pre-commit triggering as an > optimization to decrease testing wait time and limit the impact of test > flakiness [6]. But this bug in the plugin means we'd lose the ability to > manually trigger pre-commits which aren't automatically run. One workaround > would be to run the tests locally instead of on Jenkins, though that's > clearly less desirable. Is this a blocker? > > Should we: > (a) Keep pre-commit triggering enabled for now and hope the upstream patch > gets accepted, or > (b) Revert the pre-commit change and wait for the patch > > Thoughts? > > [1] https://github.com/apache/beam/pull/5611 > [2] https://github.com/apache/beam/pull/5607#issuecomment-397080770 > [3] https://github.com/apache/beam/pull/5638 > [4] https://github.com/jenkinsci/ghprb-plugin/issues/678 > [5] https://github.com/jenkinsci/ghprb-plugin/pull/680 > [6] > https://docs.google.com/document/d/1lfbMhdIyDzIaBTgc9OUByhSwR94kfOzS_ozwKWTVl5U/edit#bookmark=id.6j8bwxnbp7fr > > > On Wed, Jun 13, 2018 at 10:03 PM Rui Wang <ruw...@google.com> wrote: > >> Precommit filter is a really coooooooooool optimization! >> >> -Rui >> >> On Wed, Jun 13, 2018 at 5:21 PM Andrew Pilloud <apill...@google.com> >> wrote: >> >>> Ah, so this is intended and I didn't break anything? Cool! Sorry for the >>> false alarm, looks like a great build optimization! >>> >>> Andrew >>> >>> On Wed, Jun 13, 2018 at 5:06 PM Yifan Zou <yifan...@google.com> wrote: >>> >>>> Probably due to the precommit filter applied in #5611 >>>> <https://github.com/apache/beam/pull/5611>? >>>> >>>> On Wed, Jun 13, 2018 at 5:02 PM Andrew Pilloud <apill...@google.com> >>>> wrote: >>>> >>>>> Looks like statuses got posted between me writing this email and >>>>> sending it. Still wondering why the python and go jobs appear to be >>>>> missing? >>>>> >>>>> Andrew >>>>> >>>>> On Wed, Jun 13, 2018 at 5:00 PM Andrew Pilloud <apill...@google.com> >>>>> wrote: >>>>> >>>>>> Recent PRs don't appear to be running all the precommits, and success >>>>>> status isn't being pushed to PRs. Anyone know what is going on? >>>>>> >>>>>> See: >>>>>> https://github.com/apache/beam/pull/5592 >>>>>> https://github.com/apache/beam/pull/5622 >>>>>> >>>>>> Andrew >>>>>> >>>>>> -- ------- Jason Kuster Apache Beam / Google Cloud Dataflow See something? Say something. go/jasonkuster-feedback