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

[MNT] Add shellcheck to pre-commit #1703

Merged
merged 5 commits into from Dec 31, 2021
Merged

[MNT] Add shellcheck to pre-commit #1703

merged 5 commits into from Dec 31, 2021

Conversation

mloning
Copy link
Contributor

@mloning mloning commented Dec 5, 2021

What does this implement/fix? Explain your changes.

  • yml formatting to precommit-config
  • add shellcheck to precommit
  • update precommit
  • run shellcheck on shell scripts

Does your contribution introduce a new dependency? If yes, which one?

Just a pre-commit dependency:

Comments

@mloning mloning added the maintenance Continuous integration, unit testing & package distribution label Dec 5, 2021
@mloning
Copy link
Contributor Author

mloning commented Dec 20, 2021

This improves shell scripts, this is ready to go in - perhaps someone you could find someone to review @fkiraly?

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 20, 2021

looks straightforward, may I quickly ask to confirm: is any diff besides adding shellcheck linting/formatting related?

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 29, 2021

looks ok to me - @lmmentel, can you kindly have a look at this, whether there would be any problems with your refactor?

@lmmentel
Copy link
Contributor

Looks good to me, and it shouldn't affect anything that I've touched so far.

@fkiraly fkiraly merged commit 7f23e08 into main Dec 31, 2021
@fkiraly fkiraly deleted the add-shellcheck branch December 31, 2021 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Continuous integration, unit testing & package distribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants