ShellCheck is now running on edx-platform master

As of last week, we decided to begin running ShellCheck, a linter for shell scripts, on all new edx-platform PRs as well as master. To see its output, go to the edx-platform commit log, click the :white_check_mark: or :x: icon under any commit message, and click “ShellCheck > Details”.

The check is currently optional–you can still merge your PR if it fails. Of course, if it fails because of a real warning in a shell script, try to fix it! But if the ShellCheck output isn’t making sense, or if it seems like it’s failing when it shouldn’t be, just @ me and I’m happy to take a look.

I am currently keeping an eye on the ShellCheck runs to make sure they’re not showing any flakiness. If all goes well, I plan to make it a required check on April 3, 2023, the Monday after the Open edX conference. At that point, any PRs opened before March 10 will need to be rebased in order to get a green build.

Repository owners and maintainers: if your repository contains shell scripts that are important enough to warrant linting, you can easily add ShellCheck by copying this workflow template into .github/workflows and replacing $default-branch with main or master.

7 Likes

If the ShellCheck output isn’t making sense, or if it seems like it’s failing when it shouldn’t be

I’ve run into issues where I’ve needed to have ShellCheck ignore certain errors before. Adding a comment to the script and ignoring a specific error can be a perfectly reasonable solution (and will be easy to understand in a PR!)

1 Like

ShellCheck is now required to pass on edx-platform PRs.