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

Generic shellscripts #1144

Merged
merged 6 commits into from
Dec 29, 2021
Merged

Generic shellscripts #1144

merged 6 commits into from
Dec 29, 2021

Conversation

slycordinator
Copy link
Contributor

Modifies shell scripts to be use a more generic /bin/sh where possible with minor changes for portability and otherwise has small changes in variable quoting and such as recommended by the shellcheck utility.

The source/build.sh script has been left as a bash, as it uses a lot of array functionality that isn't present in POSIX.

Doesn't use anything bash-specific, so it can be /bin/sh
Doesn't use anything bash-specific, so it can be a /bin/sh script

Added quotes to MDPATH variable, as recommended by shellcheck utility
Doesn't use anything bash-specific, so can be a /bin/sh script
Modified quoting/variables as recommended by shellcheck
Doesn't use anything bash-specific so it can be a /bin/sh script
Change to more generic /bin/sh script

* added support for using on shells that don't have UID variable; uses UID if it's available and calls "id -u" when it's not
* modified quoting as recommended by shellcheck
* changed to not use "$?" to check for failure. Now uses "if ! (command); then (fail condition); fi" for when a command fails
@Ralim
Copy link
Owner

Ralim commented Dec 27, 2021

Hi,
Looks good to me.

Since you used shellcheck for this, can you add shellcheck to the ci actions to maintain these changes

@Ralim Ralim merged commit 53eb92c into Ralim:master Dec 29, 2021
@slycordinator
Copy link
Contributor Author

slycordinator commented Jan 2, 2022 via email

@Ralim Ralim mentioned this pull request Jan 3, 2022
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