-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
build: run ShellCheck in CI #31809
build: run ShellCheck in CI #31809
Conversation
b37023b
to
8cd2f03
Compare
7261985
to
ca6d2d5
Compare
so it seems one of the reasons this is failing because https://github.com/openedx/edx-platform/blob/master/common/test/data/static/contains.sh is shebanging
is the lack of |
That's a really old script, so I think we can just tell shellcheck ignore it. I don't think we should be committing zsh to repos for the same reason we wouldn't commit Ruby or Go: they're not Python, JS, or standard *nix tooling. |
bbd4516
to
95e7690
Compare
d6bbfac
to
0718199
Compare
3885592
to
bbdf0f5
Compare
.github/workflows/shellcheck.yml
Outdated
matrix: | ||
os: ["ubuntu", "macos"] | ||
#uses: openedx/.github/.github/workflows/shellcheck.yml@master | ||
uses: kdmccormick/openedx.github/.github/workflows/shellcheck.yml@kdmccormick/shellcheck-impl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just making a note that this needs to be updated before we merge this, I see that you have the comment ready to swap, when the .github
PR has merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder 👍🏻
285140c
to
c2c7803
Compare
This and openedx/.github#64 are both ready for another round of review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good once the upstream .github
PR merges and we do the final update to the job URL.
Additionally, this brings all existing shell scripts into compliance with ShellCheck. This implements part of an ADR, proposed here: openedx/.github#60
This reverts commit a070c30fcc52a9485d2b0ec229e0deb75a5cc958.
This reverts commit c2c78034864c42b571c87577f8dc19888a70974f.
…e_modules comment
681bb16
to
b48ecfd
Compare
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
EdX Release Notice: This PR has been deployed to the production environment. |
1 similar comment
EdX Release Notice: This PR has been deployed to the production environment. |
Description
Supporting information
This implements an ADR:
and uses a new workflow template & action:
This required removing some old, unused shell scripts which had a lot of violations:
Other Information
See openedx/.github#64 for screenshots of the build.
Commits will be squashed before merging.
Test instructions
ls $whoops
.