Shellcheck automation
carloslancha opened this issue · 10 comments
As @julien suggested here it could be a good idea to automate shell scripts check in the ci process.
https://github.com/marketplace/actions/shellcheck seems to be a good choice.
In case we decide to not automate it I will use this issue to manually format current ck.sh version.
@carloslancha thanks for opening this issue, so we can talk about it 👍
Something I didn't mention in my comment but that I had in mind, is that shellcheck won't automatically fix our code, but just check it , but I still think it's a good idea.
I've never integrated shellcheck as part of a CI or build process — but I have run it periodically. I don't know whether we should bother baking it in, or just running it by hand every so often (after we make bigger changes to the script).
I think we can close this one for now.
I saw that GitHub has a ShellCheck "action" that we could enable if we really wanted to use it.
I saw that GitHub has a ShellCheck "action" that we could enable if we really wanted to use it.
FWIW I'd vote against relying on a GitHub-specific action, because it means you can't run the check locally without creating a PR.
Also, Git itself is decentralized in spirit (like open source is, like shellcheck is), but GitHub is centralized and propietary in spirit (despite the marketing). There is a reason Microsoft paid $7.5 billion to acquire GitHub; their incentive is to lock us in... ours is to be free to move to other platforms when they are more attractive to us; Phabricator anyone?.
If you agree, please re-open @julien.
their incentive is to lock us in... ours is to be free
@wincent I agree with that, and I was proposing an easy solution (I don't know how easy it is to install shellcheck on Windows or macOS for example, on linux it's a 15MB package ... for a linter that's a lot, but whatever), so I'll re-open
That said, given that we use Slack (with bots and workflows), Jira, Google Office (sorry don't know the official name), aren't we already locked at many levels?
aren't we already locked at many levels?
Indeed we are, but that's not an argument for making things worse, especially if it imposes the requirement to create a pull request in order to check the code when all our other checks can run locally from the command-line (technically, I think this might be an example of the "Package-Deal fallacy", but I'm not sure — eg. "because we have chosen vendor lock-in on other occasions, we should do it this time too"). I also doubt a 15 MB download is a deal breaker when an existing liferay-ckeditor repo checkout is already 438 MB. A working copy of our beloved Clay repo is currently on the order of 1.2 GB (lol).
As for Windows, I imagine install it is the usual Windows pain in the ass, but if we ever get a contribution from a Windows user we can re-evaluate at that time.
that's not an argument for making things worse, especially if it imposes the requirement to create a pull request in order to check the code when all our other checks can run locally from the command-line
That's true
I also doubt a 15 MB download is a deal breaker
Also true
So I actually agree: let's think about using shellcheck and forget about the github "shellcheck" action
This is weird I have other replies in my inbox but I can't see them in GitHub's UI (things about cp -r and rsync)
Direct link might work: #124 (comment)
🤦