Fix shellcheck pre-commit hook installation error

gonzalo-bulnes commented 3 years ago

PR progress checklist (to be filled in by reviewers)

[ ] Changes to documentation are appropriate (or tick if not required)
[ ] Changes to tests are appropriate (or tick if not required)
[ ] Reviews completed

What type of PR is this?

Primary type

[ ] [build] Changes related to the build system
[x] [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
[ ] [ci] Changes to the continuous integration configuration
[ ] [feat] A new feature
[ ] [fix] A bug fix
[ ] [perf] A code change that improves performance
[ ] [refactor] A code change that neither fixes a bug nor adds a feature
[ ] [revert] A change used to revert a previous commit
[ ] [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

[ ] [docs] Documentation changes
[ ] [test] Adding missing or correcting existing tests

Does this PR introduce a `BREAKING CHANGE`?

No.

Related issues and/or pull requests

Fixes #226

Describe the changes you're proposing

The shellcheck pre-commit hook was updated upstream fixing this same issue. See https://github.com/jumanjihouse/pre-commit-hooks/pull/85 (and https://github.com/jumanjihouse/pre-commit-hooks/pull/81 for the corresponding context and details).

Additional context

the chore(pre-commit): prefix was used before, so it seemed appropriate.

gonzalo-bulnes commented 3 years ago

The build failure is due to timeouts when connecting to download.opensuse.org:

default-opensuse-leap-152-master-py3 (log)

          History:
           - [|] Error trying to read from 'http://download.opensuse.org/distribution/leap/15.2/repo/non-oss/'
           - Download (curl) error for 'http://download.opensuse.org/distribution/leap/15.2/repo/non-oss/content':
          Error code: Connection failed
          Error message: Failed to connect to download.opensuse.org port 80: Connection timed out

default-opensuse-tmbl-latest-master-py3 (log)

          History:
           - [|] Error trying to read from 'http://download.opensuse.org/tumbleweed/repo/non-oss/'
           - Timeout exceeded when accessing 'http://download.opensuse.org/tumbleweed/repo/non-oss/content'.

myii commented 3 years ago

Thanks for the (issue and) PR @gonzalo-bulnes. This will be merged soon enough. I'm just going to ask @dafyddj to have a look, in case we want to look at one of the alternative shellcheck repos. Don't worry about the openSUSE failures, that's something that's only started today (tests have been running on the platform fine just yesterday).

gonzalo-bulnes commented 3 years ago

Sounds good @myii, thank you!

gonzalo-bulnes commented 3 years ago

Hi @dafyddj!

Thanks for asking! I don't really have an opinion on the topic of pre-commit hooks.

Truth is I don't really ever use pre-commit hooks. I understand them to be a way to enforce some project conventions, but I've so far always worked in contexts where such quality control could be managed at build time, that is I mean, (just) before merging.

I tend to prefer that because I'm quite disciplined when it comes to pre-merging cleanup, and commit-time constraints get in the way of my personal workflow preferences. But you know, if that's the project convention I'm happy to use pre-commit hooks and update them, I just don't have enough context to form an opinion on one provider or the other. 🙂

If that's easier for you to manage, please feel free to close this PR. Because we're using pre-commit hooks, I am of the opinion that #226 should be fixed, but I don't have strong views on how it should be fixed, and I'd happily leave that decision to you. Does that make sense?

dafyddj commented 3 years ago

Thanks @gonzalo-bulnes for your comments and proposed contribution. I think we will change that particular hook to shellcheck-py in #228 which I was planning to do anyway, but also fixes #226.

gonzalo-bulnes commented 3 years ago

Sound good @dafyddj, thank you!

myii commented 3 years ago

Appreciate your efforts, @gonzalo-bulnes. We've got #228 pushed out to all formulas now (those that use pre-commit, that is).

saltstack-formulas / template-formula