Skip to content

Add ShellCheck linter target#2361

Merged
supersven merged 1 commit intodevelopfrom
sventennie/shellcheck-only-new-scripts
May 9, 2022
Merged

Add ShellCheck linter target#2361
supersven merged 1 commit intodevelopfrom
sventennie/shellcheck-only-new-scripts

Conversation

@supersven
Copy link
Contributor

@supersven supersven commented May 5, 2022

It lints all shell scripts that are not excluded, i.e. new ones and old ones without issues. Excluded are those that currently have issues. They should be fixed on a step-by-step basis. This conservative/careful approach is necessary due to shell scripts having neither a type checker nor tests. "Surprises" should be avoided, especially as they might
appear in stressful situations.

This PR supersedes #2310 . In that PR I learned that even simple looking quotes may lead to awful errors.

https://wearezeta.atlassian.net/browse/SQPIT-279

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • If this PR changes development workflow or dependencies, they have been A) automated and B) documented under docs/developer/. All efforts have been taken to minimize development setup breakage or slowdown for co-workers.
    schema`** to update the cassandra schema documentation.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@supersven supersven force-pushed the sventennie/shellcheck-only-new-scripts branch from 14636cb to c82b437 Compare May 5, 2022 06:52
@supersven supersven force-pushed the sventennie/shellcheck-only-new-scripts branch from c82b437 to 2772e85 Compare May 5, 2022 06:57
@supersven supersven temporarily deployed to cachix May 5, 2022 06:57 Inactive
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

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

We should create issues to fix the ignored scripts if we really want to do the FUTUREWORK.
Approving since this is still an improvement.

@supersven supersven force-pushed the sventennie/shellcheck-only-new-scripts branch from 2772e85 to e45d7e4 Compare May 5, 2022 15:12
@supersven supersven temporarily deployed to cachix May 5, 2022 15:12 Inactive
@supersven
Copy link
Contributor Author

@akshaymankar , Good idea 👍

I've created tasks/tickets for the ignored files: https://wearezeta.atlassian.net/browse/SQPIT-952

It lints all shell scripts that are not excluded, i.e. new ones and old
ones without issues. Excluded are those that currently have issues. They
should be fixed on a step-by-step basis. This conservative/careful
approach is necessary due to shell scripts having neither a type checker
nor tests. "Surprises" should be avoided, especially as they might
appear in stressful situations.
@supersven supersven force-pushed the sventennie/shellcheck-only-new-scripts branch from e45d7e4 to 109356b Compare May 9, 2022 07:15
@supersven supersven temporarily deployed to cachix May 9, 2022 07:15 Inactive
@supersven supersven merged commit 96f9937 into develop May 9, 2022
@supersven supersven deleted the sventennie/shellcheck-only-new-scripts branch May 9, 2022 09:17
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.

3 participants

Comments