-
Notifications
You must be signed in to change notification settings - Fork 8
Add switches to dataset runner scripts #67
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
base: main
Are you sure you want to change the base?
Conversation
examples/runCfhtQuickTest.sh
Outdated
| done | ||
| shift $((OPTIND-1)) | ||
|
|
||
| if [ "$DOPROCESS" = true ] ; then |
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.
Since the shebang line is invoking bash, the improved [[ ]] can be used for testing. Variables do not need to be quoted inside of [[ ]] and == may be used instead of the posix = equality operator.
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.
Will do.
examples/runCfhtQuickTest.sh
Outdated
| p) DOPROCESS=false;; | ||
| v) DOVERIFY=false;; | ||
| h) usage;; | ||
| *) usage;; |
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.
Technically, h and * don't need to be separate cases as * will match h but that might be a slightly non-obvious to future maintainers. However, it should be possible to condense these two matches with h | *).
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'm removing the * as we discussed.
| DOVERIFY=true | ||
|
|
||
| usage() { | ||
| print_error |
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 is down to personal preference but you could replace the multiple print_error calls with.
print_error "$(cat <<-EOF
EOF
)"
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'd prefer to leave this as is since it's the same as in other scripts in this repo. I'm happy to file a ticket to update the syntax in the future.
These switches allow processing and verification steps to be turned off.
56dd07c to
812a30d
Compare
These switches allow processing and verification steps to be turned off.
Note that this does not change the default behavior. The switches turn the different aspect off.