Skip to content
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 bash in scripts #194

Merged
merged 3 commits into from
Apr 21, 2021
Merged

Fix bash in scripts #194

merged 3 commits into from
Apr 21, 2021

Conversation

Sriep
Copy link
Contributor

@Sriep Sriep commented Apr 19, 2021

Changed shell scripts that make use of [[ ]] to use bash as they don't work in sh.

Modified scripts as suggested by SC2199 to not use $@ in [[.

Fixed unit_test.sh, which was not working as intended.

Resolves issue #176

@Sriep Sriep requested a review from bbist April 20, 2021 18:23
Copy link
Contributor

@bbist bbist left a comment

Choose a reason for hiding this comment

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

All changes look good.
One thing to note about docker.local/bin/unit_test.sh is that the expected order of flags and package name has changed. Previously, if a package and --sc flag were provided --sc flag was expected to come after the package name. Now --sc flag (if provided) should come before package name (if provided). I think the new one is better as it brings consistency to the usage of unit_test.sh.
If someone is using unit_test.sh locally, they may want to change the order of their flags and arguments.

@Sriep Sriep merged commit 4c19299 into master Apr 21, 2021
@Sriep Sriep mentioned this pull request Apr 21, 2021
@platsko platsko deleted the sh-bash branch May 5, 2021 10:08
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.

None yet

2 participants