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

Create some tests #80

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Create some tests #80

merged 1 commit into from
Aug 29, 2024

Conversation

laduke
Copy link
Contributor

@laduke laduke commented Aug 28, 2024

This only lints for "errors" -S error

The shebang line is injected from build-install.sh. There was probably a reason for that, so I marked it ignored.

There are a few more errors like:

In install.sh.in line 233:
        if   [[ "$DISTRIBUTION" == "ubuntu" && "$VERSION" < "22.04" ]]; then
                                                            ^-----^ SC2072 (error): Decimals are not supported. Either use integers only, or use bc or awk to compare.

if we enable warnings and info, there are ~20 things to fix. But it's style stuff like

In install.sh.in line 364:
        cat /dev/null | $SUDO zypper addrepo -t YUM -G ${ZT_BASE_URL_HTTP}redhat/el/9 zerotier
            ^-------^ SC2002 (style): Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead.

I'll cat if I want to.

@glimberg
Copy link
Contributor

nice

@laduke
Copy link
Contributor Author

laduke commented Aug 28, 2024

i am slightly afraid to touch the errors

@laduke
Copy link
Contributor Author

laduke commented Aug 28, 2024

I tried to make an action to run curl | bash on most of the distros the script supports. All the redhat type ones don't have systemd in their image, so it doesn't work. Couldn't get mint to work either. We'd need to make VMs someday.

@Squelch
Copy link

Squelch commented Aug 29, 2024

There are a few more errors like:

In install.sh.in line 233:
        if   [[ "$DISTRIBUTION" == "ubuntu" && "$VERSION" < "22.04" ]]; then
                                                            ^-----^ SC2072 (error): Decimals are not supported. Either use integers only, or use bc or awk to compare.

if we enable warnings and info, there are ~20 things to fix.

Does -lt and -gt instead of < and > produce the same error regarding version numbers? IIRC </> are for string comparisons in bash, -lt/-gt for numerals.

[edit] I'm mobile right now, so unable to verify

@laduke laduke changed the title Shellcheck Create some tests Aug 29, 2024
@laduke laduke marked this pull request as ready for review August 29, 2024 16:14
@laduke laduke force-pushed the shellcheck branch 2 times, most recently from 73d78ae to 6c69076 Compare August 29, 2024 16:39
glimberg
glimberg previously approved these changes Aug 29, 2024
.github/workflows/ci.yml Outdated Show resolved Hide resolved
Couldn't get fedora/redhat type containers to work. TODO.

Shellcheck 2070 says:
Exceptions

If the strings happen to be version numbers and you're using <, or > to compare them as strings, and you consider this an acceptable thing to do, then you can ignore this warning.

So I left it. The alternatives are complicated bash, or external tools
that aren't always installed.
Copy link
Contributor

@glimberg glimberg left a comment

Choose a reason for hiding this comment

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

:shipit:

@laduke laduke merged commit a842e3a into main Aug 29, 2024
11 checks passed
@laduke laduke deleted the shellcheck branch August 29, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants