[
https://issues.apache.org/jira/browse/BEAM-4445?focusedWorklogId=116105&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-116105
]
ASF GitHub Bot logged work on BEAM-4445:
----------------------------------------
Author: ASF GitHub Bot
Created on: 26/Jun/18 19:06
Start Date: 26/Jun/18 19:06
Worklog Time Spent: 10m
Work Description: swegner commented on issue #5757: [BEAM-4445] Split
pre-commit tests into separate jobs based on trigger condition.
URL: https://github.com/apache/beam/pull/5757#issuecomment-400428025
Reviewed 16 of 17 files at r2, 1 of 1 files at r3.
Review status: all files reviewed, 5 unresolved discussions (waiting on
@Ardagan)
---
*[.test-infra/jenkins/JobBuilder.groovy, line 48 at
r3](https://reviewable.io/reviews/apache/beam/5757#-LFwp3w_ObuCuE9yaHxq-r3-48:-LFxTVQy6FF8yddFwj06:b-896fix)
([raw
file](https://github.com/apache/beam/blob/beff3c3d0fc951cf8aaac3fdaa21de4a3d24d7e8/.test-infra/jenkins/JobBuilder.groovy#L48)):*
<details><summary><i>Previously, Ardagan (Mikhail Gryzykhin)
wrote…</i></summary><blockquote>
Since you create PrecommitBuilder, it is best to rename this class to
PostcommitBuilder.
I would also rename both classes to "xxxxJobBuilder"
</blockquote></details>
Done.
---
*[.test-infra/jenkins/PrecommitBuilder.groovy, line 70 at
r3](https://reviewable.io/reviews/apache/beam/5757#-LFtLdYtLZInn-e04zZr-r3-70:-LFxTcdX1XXs9gARIF4g:b-sx0b7w)
([raw
file](https://github.com/apache/beam/blob/beff3c3d0fc951cf8aaac3fdaa21de4a3d24d7e8/.test-infra/jenkins/PrecommitBuilder.groovy#L70)):*
<details><summary><i>Previously, Ardagan (Mikhail Gryzykhin)
wrote…</i></summary><blockquote>
It is better to either use braces in all places, or omit them in all places
uniformly. (see line 61 vs 70)
</blockquote></details>
Thanks, good catch.
---
*[.test-infra/jenkins/PrecommitBuilder.groovy, line 86 at
r3](https://reviewable.io/reviews/apache/beam/5757#-LFtLdYtLZInn-e04zZr-r3-86:-LFxTl5sBX2eAJZn29tQ:b-896fix)
([raw
file](https://github.com/apache/beam/blob/beff3c3d0fc951cf8aaac3fdaa21de4a3d24d7e8/.test-infra/jenkins/PrecommitBuilder.groovy#L86)):*
<details><summary><i>Previously, Ardagan (Mikhail Gryzykhin)
wrote…</i></summary><blockquote>
''' missing, and extra ')'
</blockquote></details>
Done.
---
*[.test-infra/jenkins/PrecommitBuilder.groovy, line 98 at
r3](https://reviewable.io/reviews/apache/beam/5757#-LFtLdYtLZInn-e04zZr-r3-98:-LFxTueU8zS6H52eGvJY:b-896fix)
([raw
file](https://github.com/apache/beam/blob/beff3c3d0fc951cf8aaac3fdaa21de4a3d24d7e8/.test-infra/jenkins/PrecommitBuilder.groovy#L98)):*
<details><summary><i>Previously, Ardagan (Mikhail Gryzykhin)
wrote…</i></summary><blockquote>
I would explicitly convert usesRegionFilter to allowRemotePoll for
readability.
</blockquote></details>
Done.
---
*[.test-infra/jenkins/PrecommitBuilder.groovy, line 119 at
r3](https://reviewable.io/reviews/apache/beam/5757#-LFtLdYtLZInn-e04zZr-r3-119:-LFxU48B8c4O_4yzpakR:b-896fix)
([raw
file](https://github.com/apache/beam/blob/beff3c3d0fc951cf8aaac3fdaa21de4a3d24d7e8/.test-infra/jenkins/PrecommitBuilder.groovy#L119)):*
<details><summary><i>Previously, Ardagan (Mikhail Gryzykhin)
wrote…</i></summary><blockquote>
I believe this comment is redundant, method name is descriptive enough.
</blockquote></details>
Done.
---
*Comments from [Reviewable](https://reviewable.io/reviews/apache/beam/5757)*
<!-- Sent from Reviewable.io -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 116105)
Time Spent: 2h 40m (was: 2.5h)
> Filter pre-commit triggering based on touched files
> ---------------------------------------------------
>
> Key: BEAM-4445
> URL: https://issues.apache.org/jira/browse/BEAM-4445
> Project: Beam
> Issue Type: Sub-task
> Components: build-system, testing
> Reporter: Scott Wegner
> Assignee: Scott Wegner
> Priority: Minor
> Labels: beam-site-automation-reliability
> Fix For: Not applicable
>
> Time Spent: 2h 40m
> Remaining Estimate: 0h
>
> This is discussed in the [Beam-Site Automation
> Reliability|https://s.apache.org/beam-site-automation] design, under
> "Pre-Commit Job Filtering"
> The proposal is to filter pre-commit job triggered on PR's based on which
> files are touched. The impact is that most PRs will only run one set of
> relevant tests, rather than all three. This will decrease test overhead and
> the impact of flaky tests.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
