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

pre-commit hook failed when committing multiple jsonnet files #543

Closed
thombashi opened this issue Jun 25, 2021 · 2 comments · Fixed by #545
Closed

pre-commit hook failed when committing multiple jsonnet files #543

thombashi opened this issue Jun 25, 2021 · 2 comments · Fixed by #545

Comments

@thombashi
Copy link
Contributor

pre-commit hook of jsonnet-lint failed when passing multiple jsonnet files.

$ pre-commit run --files a.jsonnet b.jsonnet
jsonnetfmt...............................................................Passed
jsonnet-lint.............................................................Failed
- hook id: jsonnet-lint
- exit code: 1

ERROR: only one file is allowed

Therefore, if you install the pre-commit hook, commits that involve changes to multiple jsonnet files will fail.
This is a little bit inconvenient for development.

Is there any reason to jsonnet-lint accept only one file?
May I open a pull request to change to accept multiple jsonnet files?

@sbarzowski
Copy link
Collaborator

Is there any reason to jsonnet-lint accept only one file?

No there isn't. I just haven't got to it yet.

May I open a pull request to change to accept multiple jsonnet files?

That would be great.

Doing it properly means passing multiple files here: https://github.com/google/go-jsonnet/blob/v0.17.0/linter/linter.go#L42
and adding them all to roots: https://github.com/google/go-jsonnet/blob/v0.17.0/linter/linter.go#L44 and subsequently processing them all. It's especially important to go over them all together here: https://github.com/google/go-jsonnet/blob/v0.17.0/linter/internal/types/check.go#L136. This way we avoid processing common dependencies multiple times during type checking.

If you don't have time for implementing the full solution, I would also be happy to accept the updated to the CLI which just calls linter on each file in a loop with an appropriate TODO. It's not as efficient, but it's still a step in the right direction.

@thombashi
Copy link
Contributor Author

thombashi commented Jun 27, 2021

@sbarzowski
Thank you for your answer and implementation advice.

I had tried to implement the full solution that you mentioned.
Please check that out.

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 a pull request may close this issue.

2 participants