-
Notifications
You must be signed in to change notification settings - Fork 46
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
Use trap to handle errors in test scripts #370
Use trap to handle errors in test scripts #370
Conversation
Co-authored-by: AJ Schmidt <ajschmidt8@users.noreply.github.com>
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 am confused.
The PR title is Reduce error handling verbosity in CI tests scripts
but the scripts are now longer. What verbosity is this intending to reduce?
Good point. This change doesn't have much effect on In other repositories, these changes were meant to replace the many conditional code blocks that existed previously for capturing error codes (like this file for cudf). @robertmaynard, if you want us to close this PR since |
Hey @robertmaynard, you are right. This PR was part of a larger effort across all repos, and the title applied to all the repos except rapids-cmake. Infact in this case, as you mentioned it's adding more lines. Apologies for the confusion, but the aim of the change is to use the |
Absolutely fine moving over to using trap. Let's just update the pr title |
/merge |
This PR adds a less verbose trap method, for error handling to help ensure that we capture all potential error codes in our test scripts, and works as follows:
cc @ajschmidt8