Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.travis: Convert TRAVIS_COMMIT_RANGE base...head to base..head #216

Merged
merged 3 commits into from Jan 24, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented Oct 6, 2015

Work around travis-ci/travis-ci#4596 until that is fixed upstream.
This avoids pulling in commits from the base tip that aren't reachable
from the head tip (e.g. if master has advanced since the PR branched
off, and the PR is against master). We only want to check commits
that are in the head branch but not in the base branch (more details
on the range syntax here).

Once the Travis bug does get fixed, the shell replacement will be a
no-op. So we don't have to worry about checks breaking once the bug
gets fixed, and can periodically poll the bug and remove the
workaround at out leisure after the fix.

Spun off from #182 now that #215 has landed.

@vbatts
Copy link
Member

vbatts commented Jan 4, 2016

is this really an issue to us?

@wking
Copy link
Contributor Author

wking commented Jan 4, 2016

On Mon, Jan 04, 2016 at 10:39:14AM -0800, Vincent Batts wrote:

is this really an issue to us?

It's only an issue if there are broken commits in the master branch.
We seem to have avoided that so far, with the only merged failure
being the (Travis hiccup?) [1,2]. So not a critical fix, but still
something that I think is worth landing.

@vbatts
Copy link
Member

vbatts commented Jan 13, 2016

as this has not impacted us, and may not for some foreseeable future, I'm inclined to close it. It could always be reopened if the need be.

@cyphar
Copy link
Member

cyphar commented Jul 22, 2016

Time to reopen I guess.

@cyphar cyphar reopened this Jul 22, 2016
@vbatts
Copy link
Member

vbatts commented Jul 25, 2016

@cyphar what is needing the reopening?

@wking
Copy link
Contributor Author

wking commented Jul 25, 2016

On Mon, Jul 25, 2016 at 11:54:45AM -0700, Vincent Batts wrote:

@cyphar what is needing the reopening?

#520 was reporting errors in old master commits 1, but its
replacement #521 did not. So you can probably re-close this and kick
the can down the road again ;).

@vbatts
Copy link
Member

vbatts commented Jul 25, 2016

😕

@wking
Copy link
Contributor Author

wking commented Jul 25, 2016

On Mon, Jul 25, 2016 at 12:02:13PM -0700, Vincent Batts wrote:

😕

I still think this is a useful change, and could land now. You
(presumably) still think we should wait until we are impacted by not
landing it 1. We may have been impacted by it in #520 2, but
that's no longer open. So we're back to “no currently open PRs are
impacted by this not being merged”, and the last time we were there
you closed the PR 1.

@vbatts
Copy link
Member

vbatts commented Jan 18, 2017

lets give this a try. could you rebase?

@RobDolinMS RobDolinMS added this to the 1.0.0 milestone Jan 18, 2017
@wking
Copy link
Contributor Author

wking commented Jan 18, 2017 via email

@wking wking force-pushed the travis-test-branch-commits branch 2 times, most recently from a7d8845 to 8952a6b Compare January 19, 2017 04:46
Work around travis-ci/travis-ci#4596 until that is fixed upstream [1].
This avoids pulling in commits from the base tip that aren't reachable
from the head tip (e.g. if master has advanced since the PR branched
off, and the PR is against master).  We only want to check commits
that are in the head branch but not in the base branch (more details
on the range syntax in [2]).

Once the Travis bug does get fixed, the shell replacement will be a
no-op.  So we don't have to worry about checks breaking once the bug
gets fixed, and can periodically poll the bug and remove the
workaround at out leisure after the fix.

[1]: travis-ci/travis-ci#4596
[2]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Jan 19, 2017 via email

Only use the auto-ranging when Travis tells us what the range is.  Use
our EPOCH_TEST_COMMIT-based range in all other cases.

ifdef is described in [1].

[1]: https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html

Signed-off-by: W. Trevor King <wking@tremily.us>
To make make debugging Travis environment issues more straightforward.

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
Copy link
Contributor Author

wking commented Jan 24, 2017

I've also brought over the new commits from opencontainers/image-spec#521, which should help with failures from local-branch PRs (e.g. here for #661).

@vbatts
Copy link
Member

vbatts commented Jan 24, 2017

LGTM

Approved with PullApprove

1 similar comment
@crosbymichael
Copy link
Member

crosbymichael commented Jan 24, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 3297cd5 into opencontainers:master Jan 24, 2017
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jan 27, 2017
Through 3297cd5 (Merge pull request opencontainers#216 from
wking/travis-test-branch-commits, 2017-01-24).

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking wking deleted the travis-test-branch-commits branch January 27, 2017 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants