-
Notifications
You must be signed in to change notification settings - Fork 60
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
Fix legacy tests #510
Fix legacy tests #510
Conversation
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.
Good catch. Not sure I understand the diff though or why it worked before. Previously it got 3 arguments, so how did it end up reading the default example_frame.root
?
It's the default value and it's changed based on the number of arguments: |
Ah, its a |
Yep, I added the check |
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.
Thanks.
closing and re-opening to trigger actions, which seem to have gone missing somehow for the last commit. |
BEGINRELEASENOTES
example_frame.root
was being used which (almost) always exists (because there is a another test creating it) so it was hard to notice.ENDRELEASENOTES
Maybe this default should be changed so as to be more explicit so this can't happen but it would make the implementation in the CMakeLists in root_io a bit less clean.