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

Add shellcheck #1843

Merged
merged 1 commit into from Feb 22, 2024
Merged

Add shellcheck #1843

merged 1 commit into from Feb 22, 2024

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Feb 16, 2024

Status

Ready for review

Description

Copy the shellcheck.sh script from the SecureDrop server repository and add it to the central make lint target. I added SC2164 as a global exclusion since we don't really care about cd potentially failing.

The following fixes are being made:

  • client/.githooks/pre-commit: Use $() instead of backticks.
  • debian/setup-venv.sh: Ignore quoting words, all the variables are a single word only.
  • proxy/entrypoint.sh: Adjust shebang since we use bashisms.
  • scripts/build-debs.sh: Set variable and then export it
  • scripts/fixup-changelog.sh: Quote our variables, don't use ! -z.

Fixes #814.

Test Plan

  • CI passes
  • Visual review

Copy link
Contributor

@rocodes rocodes left a comment

Choose a reason for hiding this comment

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

Minor comment about shellcheck exclusions that we don't actually need to exclude (and a couple we may eventually want to fix rather than ignoring) but broadly LGTM.

scripts/shellcheck.sh Show resolved Hide resolved
scripts/shellcheck.sh Outdated Show resolved Hide resolved
Copy the `shellcheck.sh` script from the SecureDrop server repository
and add it to the central `make lint` target. I added SC2164 as a global
exclusion since we don't really care about `cd` potentially failing.

The following fixes are being made:
* client/.githooks/pre-commit: Use $() instead of backticks.
* debian/setup-venv.sh: Ignore quoting words, all the variables are a
  single word only.
* proxy/entrypoint.sh: Adjust shebang since we use bashisms.
* scripts/build-debs.sh: Set variable and then export it
* scripts/fixup-changelog.sh: Quote our variables, don't use `! -z`.

Fixes #814.
Copy link
Contributor

@rocodes rocodes left a comment

Choose a reason for hiding this comment

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

:shipit:

@rocodes rocodes merged commit caef187 into main Feb 22, 2024
72 checks passed
@rocodes rocodes deleted the shellcheck branch February 22, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add shellcheck
2 participants