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: lint shell script files #3068

Merged
merged 6 commits into from
Apr 7, 2019
Merged

Conversation

oliver-sanders
Copy link
Member

Run the brilliantly named shellcheck over all shell scripts to improve code quality and cut down on review overheads.

  • Sadly shellcheck runs over files rather than projects unlike many other linters (e.g. pycodestyle) so I've implemented a minimal script for including / excluding files in the check. I did look at borrowing the tox interface from pycodestyle but it would probably be more effort than it's worth for this simple linter.
  • The test-battery scripts have a somewhat larger volume of warnings than I'm prepared to address in this PR so I've run them separately in a more lenient manner for the moment.

@oliver-sanders oliver-sanders added this to the cylc-8.0.0 milestone Apr 3, 2019
@oliver-sanders oliver-sanders self-assigned this Apr 3, 2019
@oliver-sanders oliver-sanders modified the milestones: cylc-8.0.0, cylc-8.0a1 Apr 3, 2019
Copy link
Contributor

@matthewrmshin matthewrmshin left a comment

Choose a reason for hiding this comment

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

A few comments.

.travis/shellchecker Outdated Show resolved Hide resolved
.travis/shellchecker Outdated Show resolved Hide resolved
bin/cylc Show resolved Hide resolved
bin/cylc Outdated Show resolved Hide resolved
bin/cylc-make-docs Outdated Show resolved Hide resolved
etc/dev-bin/updatecopyright.sh Show resolved Hide resolved
@oliver-sanders
Copy link
Member Author

oliver-sanders commented Apr 4, 2019

On Hold

This PR makes bash4 a requirement which will not work on our site.

@hjoliver hjoliver added the BLOCKED This can't happen until something else does label Apr 4, 2019
@hjoliver
Copy link
Member

hjoliver commented Apr 4, 2019

On Hold

This PR makes bash4 a requirement which will not work on our site.

Added a scary BLOCKED label.

.travis/shellchecker Show resolved Hide resolved
doc/src/graphics/scale-images.sh Outdated Show resolved Hide resolved
@oliver-sanders
Copy link
Member Author

Removing the "scary BLOCKED label", the only bash4 syntax is in .travis/shellchecker which is only run by Travis.

@oliver-sanders oliver-sanders removed the BLOCKED This can't happen until something else does label Apr 5, 2019
@hjoliver hjoliver merged commit abb19d1 into cylc:master Apr 7, 2019
@oliver-sanders oliver-sanders deleted the shellcheck branch May 27, 2020 11:27
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.

5 participants