Skip to content

Conversation

@aw-was-here
Copy link
Contributor

No description provided.

Copy link
Member

@ndimiduk ndimiduk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skimmed over the change, looks reasonable to me.

if ! verify_command "shellcheck" "${SHELLCHECK}"; then
add_vote_table_v2 0 shellcheck "" "Shellcheck was not available."
delete_test shellcheck
return 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the other branches below need an explicit return value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. I made this return explicit to match the other prechecks. Being explicit, it re-enforces that test-patch really shouldn't fail if a particular tool isn't around and we definitely don't want the rest of the code to continue since that has a few side-effects.

@aw-was-here
Copy link
Contributor Author

Thanks for the review @ndimiduk ! I'll go ahead and commit this one since we know why the action test is failing.

@aw-was-here aw-was-here merged commit d1c3eaa into apache:main Apr 19, 2022
@aw-was-here aw-was-here deleted the yetus1150 branch April 19, 2022 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants