Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs: How to use pre-commit-terraform image to run pre-commit in CI #656
docs: How to use pre-commit-terraform image to run pre-commit in CI #656
Changes from 3 commits
80ed46b
6d3298c
407aa20
ff7bd8b
ada52e6
e7835c4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Okay, before we move forward, I'd like to have the answer to the next question:
Would you like to add a fully functional replacement for the current GH workflow[1], or just show the possibility of docker image usage in GHA[2]?
Btw, [1] little bit related to #373
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.
[2] - show both ways seems to be a good idea to me.
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.
I guess the
.github/workflows/pre-commit.yaml
is among active workflows in this repo and hence running the same thing twice might not be desired 🤔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.
Yeah. If just show that it is possible, please:
.github/workflows/pre-commit.yaml
.github/workflows/pre-commit.yaml
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.
This would mean, that you don't actively check the current container image within GHA.
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.
According to your answer to my question above, you not plan to provide a " fully functional replacement for the current GH workflow[1]"
And we definitely won't to decrease the current test coverage. Current realization in this PR will fail on
hadolint
,shfmt
, andshellcheck
hooks as there are no such dependencies inside Docker image. (and these checks are vital for.sh
and.dockerfile
)And also, it will not push fixes back to branch.
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.
FYI: The install of additional tools should also work in the container version.
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.
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.
@MaxymVlasov : but fixes will be not automatically pushed back to your branch (not sure about the gist of
when
clause though)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.
I don't see, where the original workflow pushes changes found by pre-commit? Or do you want to be just very specific?
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.
I hope this PR and screencast will help
#658
Screencast from 15.04.24 18:59:50.webm
Push fixes part in GHA:
https://github.com/antonbabenko/pre-commit-terraform/actions/runs/8692274320/job/23836525488?pr=658#step:10:133
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.
pre-commit-terraform/.github/workflows/pre-commit.yaml
Lines 42 to 56 in 42919f3
There 2 pre-commit runs.
The first one, is able to push fixes. second-one - not. The difference in availability of
GITHUB_TOKEN
.That's done in this way because otherwise, hooks without fixes will produce exit code 1, and that stops from pushing fixed to the branch.
I can say GHA "ignore exit code 1", and fixes will be successfully pushed, but then if something will be detected by hooks that have no autofixes - there will be no notification about that, except inside logs in "successfully done" GHA
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.
I missed, that you don't use the latest version of the pre-commit/action which has no push support anymore.
So, if the 2nd run doesn't push changes, wouldn't this be a good candidate to be replaced by the container version?
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.
Still no
hadolint
dependency inside image. It will check nothing frompre-commit-hooks
(all of them, which we use here have autofixes or at least do not break the first check when fail) nor pre-commit-terraform repo hooks (as this repo doesn't include any tf code for testing).If you'd like to make useful tests, check #373.