-
Notifications
You must be signed in to change notification settings - Fork 382
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 pre-commit-hooks+config to use with pre-commit.com #1879
base: main
Are you sure you want to change the base?
Conversation
And self-apply in github workflows.
# Github image should be /usr/bin/pwsh | ||
# Linux/Macos is usually /usr/local/bin/pwsh unless using a user scope | ||
entry: /usr/bin/pwsh -Command "Invoke-ScriptAnalyzer -Settings PSGallery -Recurse -ReportSummary -EnableExit -Severity Warning -Path . " | ||
# https://github.com/pre-commit/pre-commit/pull/2340#issuecomment-1098203344 |
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.
shouldn't this just be removed?
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.
If removed, should be somewhere in docs (where?) as user may need to adapt powershell path
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 mind adding a CI enforcement check but against commit hooks using commit hooks because PSSA is a binary module and this would mean I'd need a separate shell for committing versus local testing.
Current CI setup seems to not be working anyway because when you run invoke-scriptanalyzer on repo like you do there ARE violations (which would need to be fixed first) , even when excluding the Tests
folder, which would be expected as well as running the default set of rules and not just the PSGallery ones.
It's up to you if want CI with warning(continue-on-error) or not and if violation fixes should go into this PR too. if small or exclusions, certainly. if bigger, probably better separate. Only warnings here but exit code is equivalent to error. |
Hi There, Do we have any news about it? |
And self-apply in github workflows.
PR Summary
This allows easy integration with pre-commit that can be use to validate/lint code before commit (and also after/server side as per github workflow example).
Tested on Macos and Linux.
Note that current workflow fail is related to warnings as seen on https://github.com/juju4/PSScriptAnalyzer/actions/runs/3867323454/jobs/6592019858#step:5:3554
can change severity to errors if prefer.
Further improvement discussion in pre-commit/pre-commit#2645
PR Checklist
.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.