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

test_runner: Remove travis specific code #14630

Merged
merged 1 commit into from Nov 2, 2018

Conversation

Projects
None yet
5 participants
@MarcoFalke
Copy link
Member

commented Nov 1, 2018

The tests are no longer run on travis, but in a docker, developer machines or a windows vm.

The code was essentially dead for months now. Fix that by explicitly passing in --ci to the test runner on our docker and appveyor windows vm.

@laanwj laanwj added the Tests label Nov 1, 2018

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

makes sense — concept ACK

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1811-testNoTravis branch Nov 1, 2018

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1811-testNoTravis branch Nov 1, 2018

@jnewbery

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Concept ACK.

Why so many style-only changes in the same commit?

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Nov 1, 2018

Passing the value through touches a lot of function signatures and I run a python formatter on all changes I make before submitting a pull request. In the hope that no one can complain about my code style...

$ which yfd
alias yfd='PATH=/home/marco/workspace/yapf-diff/virt_env_3_yapf/bin python3 /home/marco/workspace/yapf-diff/yapf-diff.py -p1 -i'

https://github.com/MarcoFalke/yapf-diff

@jnewbery

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

A lot of it seems like your personal preference for style, and it inflates the diff substantially. For example, I don't think there's project guidelines about whether continuation lines of long function calls should be aligned to the opening parens or to some other indentation (both are fine under PEP8 https://www.python.org/dev/peps/pep-0008/#indentation).

This seems like a +-5 line diff without the style changes. At the least, can you split the style changes into a separate commit so it's obvious what reviewers are supposed to be paying attention to?

@MarcoFalke MarcoFalke force-pushed the MarcoFalke:Mf1811-testNoTravis branch to fa43626 Nov 1, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member Author

commented Nov 1, 2018

Removed the style change that affected lines that were in proximity to actually changed lines.

@ken2812221
Copy link
Member

left a comment

utACK fa43626

@jnewbery

This comment has been minimized.

Copy link
Member

commented Nov 2, 2018

utACK fa43626

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Nov 2, 2018

Merge bitcoin#14630: test_runner: Remove travis specific code
fa43626 test_runner: Remove travis specific code (MarcoFalke)

Pull request description:

  The tests are no longer run on travis, but in a docker, developer machines or a windows vm.

  The code was essentially dead for months now. Fix that by explicitly passing in `--ci` to the test runner on our docker and appveyor windows vm.

Tree-SHA512: 5d48693c03e8eb27536658ccf9ba738fe93a72abd4b72c80caac084b5b2cdffa77a1031a671eeefe70b71d63500f55917803d4be54d01849722afdccb700a9e6

@MarcoFalke MarcoFalke merged commit fa43626 into bitcoin:master Nov 2, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1811-testNoTravis branch Nov 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.