Skip to content
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

feat: add actionlint hook and fix current errors #505

Merged
merged 3 commits into from
May 21, 2023
Merged

Conversation

behnazh
Copy link
Collaborator

@behnazh behnazh commented Mar 21, 2023

This PR adds actionlint as a pre-commit hook to lint and check GitHub Actions workflows. The check runs using make check-actionlint.

The new actionlint hook detects errors that were also addressed in #482 and #493, which had to be fixed here for the workflows to succeed.

@behnazh behnazh requested a review from jenstroeger March 21, 2023 23:03
@behnazh
Copy link
Collaborator Author

behnazh commented Mar 21, 2023

Ah, I forgot again that we don't create venv in GitHub Actions 🤦

@behnazh behnazh marked this pull request as draft March 21, 2023 23:06
@behnazh behnazh force-pushed the add-actionlint branch 2 times, most recently from aa57c5b to 96392ac Compare March 27, 2023 22:50
@jenstroeger
Copy link
Owner

Duplicates #493, fixes #485

@behnazh behnazh force-pushed the add-actionlint branch 2 times, most recently from c486a9d to b9c71a3 Compare May 1, 2023 22:32
@jenstroeger
Copy link
Owner

jenstroeger commented May 15, 2023

What about something like actionlint-py and shellcheck-py?

@behnazh behnazh marked this pull request as ready for review May 15, 2023 12:58
@behnazh
Copy link
Collaborator Author

behnazh commented May 15, 2023

What about something like actionlint-py and shellcheck-py?

Thanks. I knew about shellcheck-py but didn't know about actionlint-py. I switched to actionlint-py , see commit 1bf019e.

I'm not sure if adding shellcheck-py makes sense in this repo though because we don't have any bash scripts 🤔

@jenstroeger
Copy link
Owner

I'm not sure if adding shellcheck-py makes sense in this repo though because we don't have any bash scripts 🤔

I think it does: actionlint can use shellcheck to check the job scripts, provided shellcheck is available (docs).

@behnazh
Copy link
Collaborator Author

behnazh commented May 15, 2023

I'm not sure if adding shellcheck-py makes sense in this repo though because we don't have any bash scripts thinking

I think it does: actionlint can use shellcheck to check the job scripts, provided shellcheck is available (docs).

I have already made a note on that in the README.md here. Not sure if you had seen it?

Copy link
Owner

@jenstroeger jenstroeger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @behnazh! Two minor comments follow…

.github/workflows/sync-with-upstream.yaml Outdated Show resolved Hide resolved
.github/workflows/sync-with-upstream.yaml Outdated Show resolved Hide resolved
@behnazh behnazh merged commit 8be21a5 into staging May 21, 2023
@jenstroeger jenstroeger deleted the add-actionlint branch July 19, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants