Shellcheck automation

[carloslancha]

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.

[julien]

@carloslancha thanks for opening this issue, so we can talk about it :+1:

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.

[wincent]

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).

[julien]

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.

[wincent]

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.

[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?

[wincent]

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.

[julien]

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

[julien]

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)

[wincent]

Direct link might work: https://github.com/liferay/liferay-ckeditor/pull/124#issuecomment-682417117

[julien]

:facepalm: