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

main: Explicit -range implies -no-travis #13

Merged
merged 1 commit into from Jan 27, 2017

Conversation

wking
Copy link
Contributor

@wking wking commented Jan 27, 2017

If someone went through the trouble to set -range, they probably don't want you clobbering their value with something from a TRAVIS_* environment variable.

This is why the /push test is failing in opencontainers/runtime-spec#671.

If someone went through the trouble to set -range, they probably don't
want you clobbering their value with something from a TRAVIS_*
environment variable.
@vbatts
Copy link
Owner

vbatts commented Jan 27, 2017

Looks good. CI failure is do to go1.5 and golint. I'll fix that later

@vbatts vbatts merged commit 3fd57e3 into vbatts:master Jan 27, 2017
wking added a commit to wking/git-validation that referenced this pull request Mar 21, 2017
Master builds only have a 'git clone ...' [1] so FETCH_HEAD isn't
defined and git-validation crashes [2].  This commit partially reverts
8a12a8f (main: default travis commit range is unreliable, 2017-03-21,
vbatts#13) to avoid that crash.  If TRAVIS_COMMIT_RANGE is unset [3],
falling back to TRAVIS_COMMIT may be fine.

The ... -> .. replacement works around travis-ci/travis-ci#4596 until
that is fixed upstream [4].  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 [5]).

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]: https://travis-ci.org/opencontainers/runc/jobs/213508696#L243
[2]: https://travis-ci.org/opencontainers/runc/jobs/213508696#L347
[3]: opencontainers/runc#1378 (comment)
[4]: travis-ci/travis-ci#4596
[5]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/git-validation that referenced this pull request Mar 21, 2017
Master builds only have a 'git clone ...' [1] so FETCH_HEAD isn't
defined and git-validation crashes [2].  This commit partially reverts
8a12a8f (main: default travis commit range is unreliable, 2017-03-21,
vbatts#13) to avoid that crash.  If TRAVIS_COMMIT_RANGE is unset [3],
falling back to TRAVIS_COMMIT may be fine.

The ... -> .. replacement works around travis-ci/travis-ci#4596 until
that is fixed upstream [4].  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 [5]).

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]: https://travis-ci.org/opencontainers/runc/jobs/213508696#L243
[2]: https://travis-ci.org/opencontainers/runc/jobs/213508696#L347
[3]: opencontainers/runc#1378 (comment)
[4]: travis-ci/travis-ci#4596
[5]: http://git-scm.com/docs/gitrevisions#_specifying_ranges

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking wking deleted the explicit-range-implies-no-travis branch March 21, 2017 21:59
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

2 participants