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

ci, lint: Remove usage of TRAVIS_COMMIT_RANGE #20071

Merged
merged 3 commits into from
Oct 4, 2020

Conversation

fjahr
Copy link
Contributor

@fjahr fjahr commented Oct 3, 2020

This is causing problems again, very similar to #19654.

UPDATE: This now removes all remaining usages of TRAVIS_COMMIT_RANGE and instead uses TRAVIS_BRANCH for the range, including lint-git-commit-check where TRAVIS_COMMIT_RANGE had already been removed. For builds triggered by a pull request, TRAVIS_BRANCH is the name of the branch targeted by the pull request. In the linters there is still a fallback that assumes master as the target branch.

@hebasto
Copy link
Member

hebasto commented Oct 3, 2020

Concept ACK.

As this is the second similar issue, it is natural to wonder whether other TRAVIS_COMMIT_RANGE use cases are safe?

@fjahr
Copy link
Contributor Author

fjahr commented Oct 3, 2020

Concept ACK.

As this is the second similar issue, it is natural to wonder whether other TRAVIS_COMMIT_RANGE use cases are safe?

The only other place seems to be the whitespace linter, I am looking into it :) Since this is an acute issue I just submitted it alone so it could be merged faster.

@fjahr fjahr changed the title ci: Don't use TRAVIS_COMMIT_RANGE for commit-script-check ci, lint: Remove usage of TRAVIS_COMMIT_RANGE Oct 3, 2020
@hebasto
Copy link
Member

hebasto commented Oct 3, 2020

@fjahr

While you are working on this: detailed comments will be much appreciated :)

ci/lint/06_script.sh Outdated Show resolved Hide resolved
@fjahr
Copy link
Contributor Author

fjahr commented Oct 3, 2020

@fjahr

While you are working on this: detailed comments will be much appreciated :)

True, I added some on TRAVIS_BRANCH and a little note where I still use MERGE_BASE. Let me know if you see other places where I should add some more.

@sipa
Copy link
Member

sipa commented Oct 3, 2020

ACK a91ab86. See test I tried in #20075.

The AppVeyor failure is expected, see #20066.

@maflcko maflcko merged commit 06314fb into bitcoin:master Oct 4, 2020
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants