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

Shellcheck #1139

Closed
wants to merge 10 commits into from
Closed

Shellcheck #1139

wants to merge 10 commits into from

Conversation

Alphakilo
Copy link
Collaborator

In an effort to make the arkmanager more robust, I've let it lint by shellcheck, got rid of all "errors" and implemented some of their hints in different warnings.

On top of that, I've set up a Github action that runs shellcheck on every push. It'll be nagging a lot, since I've configured the "error" threshold as "warning" (see f5312ee), but I'll go ahead and change more things.

@@ -1748,6 +1751,8 @@ isUpdateCancelRequested(){
xargs -0 grep -F -e "${chatCommandRestartCancel}" |
sed 's@^[[]\(....\)\.\(..\)\.\(..\)-\(..\)\.\(..\)\.\(..\):.*@\1-\2-\3 \4:\5:\6 UTC@' |
head -n1)"
# canceltime is a command, not a string
# shellcheck disable=SC2157
if [ -n canceltime ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think canceltime should be "${canceltime}"

@klightspeed
Copy link
Collaborator

I like it.

@Alphakilo
Copy link
Collaborator Author

Alphakilo commented Aug 19, 2020

On another note: can I persuade netinst.sh or the updater to the branch on my fork, so I can run some tests?

I've noticed the variables are there, but I have not figured out how to parameterize those.

@klightspeed
Copy link
Collaborator

I have added a --repo=* parameter to netinstall.sh, so you can specify --repo=Alphakilo/ark-server-tools

@klightspeed
Copy link
Collaborator

I have now implemented the idea in this pull request.

@klightspeed klightspeed closed this Aug 9, 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.

None yet

2 participants