-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Halt execution when invalid options are provided #76
Conversation
Signed-off-by: Alfredo Deza <adeza@anchore.com>
9724466
to
3d88dea
Compare
Signed-off-by: Alfredo Deza <adeza@anchore.com>
Signed-off-by: Alfredo Deza <adeza@anchore.com>
Signed-off-by: Alfredo Deza <adeza@anchore.com>
Signed-off-by: Alfredo Deza <adeza@anchore.com>
JOB=$1 | ||
|
||
# ensure this is at the root | ||
cd "$SCRIPTPATH/.." |
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.
nit / clarifying question: if this script is only meant to run in the root of the repo, would $(pwd)
be alright instead?
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.
interesting, yes, I think that would work. I did it this way to ensure that regardless of where it runs it is always correct. Do you think that assurance is overkill?
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 looks great, really like the direction you took!
core.setFailed( | ||
"At least one source for scanning needs to be provided. Available options are: image, and path" | ||
); | ||
throw new Error("At least one source for scanning needs to be provided. Available options are: image, and path"); |
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.
(no change needed) I took another look at this (re our Zoom chat the other day) and it makes sense why this was needed. Throwing an error accomplishes failing the job and skipping the remainder code, only because of the try/catch setup here. Another option is to control the flow via an early return, which lets you control the execution separately from deciding the job status if you ever need to. That might be of interest if we want to move away from large try blocks one day 😃
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.
That is a good point. I will open a ticket to track this improvement
Closes #67
act
make check
which in turn runs all the new test harness