-
Notifications
You must be signed in to change notification settings - Fork 38
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
The flag '--fix' in default pre-commit-hook entry #1139
Comments
Evening @larshb , Thinking about this a little more, it would seem the I believe the following would work: - id: vsg
name: fix VHDL style
description: VHDL Style Guide
entry: vsg --jobs=1
language: python
types:
- file
files: \.(vhd|vhdl)$ I would suggest using the I pushed an update to the Adding @alonbl who developed the pre-commit-hook for comments. Regards, --Jeremy |
Hello,
This is the output one gets, it is self explanatory:
If files were modified it only means developer pushed without checking, it does not important which as developer will run this on his workstation and fix everything before pushing the project (properly now) to origin.
Just for the record my - repo: https://github.com/jeremiah-c-leary/vhdl-style-guide
rev: 3.22.0
hooks:
- id: vsg
args:
- --configuration=.vsg.yaml
- --output_format=syntastic I believe the pre-commit settings and vsg behavior is comply with the pre-commit best practices, there is no reason to modify anything. Having written that, you may consider adding a optional parameter such as Regards, |
Morning,
Maybe add a
The
There are several possible outcomes when running with
So there are two cases in which a file will not be modified and in one of the cases a violation was detected but the file was not modified. In the case where only unfixable violations exist, the exit code would be 1 and the violations would be reported. Depending on the output format chosen you might have to wade through a bunch of output to find the violations since git would not detect files where were not modified.
For CI, there is the option of using I believe @larshb is requesting - id: vsg
name: fix VHDL style
description: VHDL Style Guide
entry: vsg --jobs=1
args:
- --fix
language: python
types:
- file
files: \.(vhd|vhdl)$ Regards, --Jeremy |
Hi,
I do not think it is necessary, nor it is
As you can see the default of
Correct.
This will result in adding |
First of all, thank you for the suggestions on I did not realize in-place formatting is default and expected behavior for pre-commit-hooks. We have other hooks that require an argument for this, such as - repo: https://github.com/pocc/pre-commit-hooks
rev: v1.3.5
hooks:
- id: clang-format
stages: [pre-commit]
args:
- -i , where We'll have to reconsider our You may close this issue, if you deem it not to be a bug. |
@larshb I suggest to use tox to invoke the hook, the following is a simple [tox]
skipsdist = True
envlist = style
[testenv:style]
deps =
pre-commit
commands =
pre-commit run --all-files {posargs} people run tox locally to perform the tests and CI runs the same, tox is available as system package in most distributions. |
Morning @larshb ,
Given the discussion thread I will close this issue. Regards, --Jeremy |
Environment
Describe the bug
The pre-commit-hook run in CI lists files changed and successfull executions with --fix, but gives no information about which files actually changed (ie. 0 warnings, 0 errors).
Running pre-commit locally also changes the files that need formatting instead of simply failing if any staged files needs formatting.
To Reproduce
Steps to reproduce the behavior:
vsg --fix
on all modified VHDL-files, but 0 warnings and 0 errors in logExpected behavior
For pre-commit (and especially in linting in CI) we expect to see which files are failing and why, not a list of all changed files with 0 warnings and 0 errors without any information on why the CI lint job fails.
Screenshots
Additional context
--fix
should be optionally added to pre-commit-configargs:
, not in the default configuration'sentry:
The text was updated successfully, but these errors were encountered: