-
Notifications
You must be signed in to change notification settings - Fork 138
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
Fixup and enhance invocation of run_tests.sh #434
Conversation
ATM it would complain that rc is not legit command due to all those spaces. Because no set -e used it just continues. Also made it executable and adjusted shebang to be universaly working. There should be no need to invoke directly via bash -- shebang would do that
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.
to the extend that I understand, I approve
So the honors would go to @effigies for the last look/click of the green button! ;) |
| [synthetic](https://github.com/bids-standard/bids-examples/tree/master/synthetic) | A synthetic dataset | anat, beh, func | T1w, beh, bold, events, physio, scans, sessions, stim | n/a | [@effigies](https://github.com/effigies) | |
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 this table is generated automatically. If it will get reverted with the next regeneration, then it would be better to fix up the script.
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 that's true
though I cannot see what actually got changed: I suspect a whitespace so I don't think it matters much
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.
probably "no newline" at the end of file?
bids-examples/tools/print_dataset_listing.py
Line 129 in 711be5b
def add_tables(df: pd.DataFrame, output_file: Path) -> None: |
And it got auto-inserted by the editor the last person touching this file was using?
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.
Otherwise this is fine. @Remi-Gau I defer to you on the README.md.
ATM it would complain that rc is not legit command due to all those spaces. Because no set -e used it just continues.
Also made it executable and adjusted shebang to be universaly working. There should be no need to invoke directly via bash -- shebang would do that
Script likely worked correctly anyways since there is explicit
exit $rc
so if$rc
remained undefined and no failed validator runs, it was justexit
- clean exit?