-
Notifications
You must be signed in to change notification settings - Fork 38.7k
ci: Move travis workarounds to .travis.yml #19276
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
Conversation
6e84e89 to
6fbe3b2
Compare
6fbe3b2 to
fa71667
Compare
|
Has anyone an explanation? |
| SHELL_OPTS="CONFIG_SHELL=" | ||
| fi | ||
| # Temporary workaround for https://github.com/bitcoin/bitcoin/issues/16368 | ||
| python3 -c 'import time; [print(".") or time.sleep(500) for _ in range(4)]' & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the issue was that this print did nothing, because by default flush=False (see https://docs.python.org/3.8/library/functions.html#print ).
Testing locally and on travis x86, it did always happen to print, just not on travis arm.
| bash -c "${CI_WAIT}" & # Print dots in case the tests take a long time to run | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This deletion is also necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI_WAIT no longer exists and this should be a no-op
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, CI_WAIT was dropped from the .travis.yml in #18929.
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fa71667, I have reviewed the code and it looks OK, I agree it can be merged.
It seems odd to have travis related workarounds in the general ci config files. Fix that oddity by moving the travis related workarounds to the travis yaml file.
For unexplained reasons, this should also work around and thus close #19171