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.com 559 #114

Merged
merged 2 commits into from
Oct 20, 2021
Merged

Conversation

jrottenberg
Copy link
Contributor

To work toward completion of pre-commit/pre-commit.com#559

@armsnyder
Copy link
Contributor

armsnyder commented Oct 11, 2021

Do we assume all of these examples are generated using the same helm-docs arguments? I recently added an example in a PR #112 where I used a --document-dependency-vlaues arg.

@jrottenberg
Copy link
Contributor Author

Comment on the wrong PR ?

@jrottenberg
Copy link
Contributor Author

@norwoodj @skang0601 any feedback ?

@skang0601
Copy link
Collaborator

@jrottenberg Sorry I'm not too familiar with the language config and what it's doing here.

So this executes a go get when a user uses this hook?

@jrottenberg
Copy link
Contributor Author

jrottenberg commented Oct 20, 2021

Yep exactly, I've kept the original hook, since it is in used already (and popular), just adding the ability to install the hook and have it execute properly even if you don't have the tool installed , as long as you have golang toolchains (pre-commit does the install and cache under tho hood)

Did a quick demo :
asciinema play -i 0.1 https://asciinema.org/a/443563

I've edited the output of cat .pre-commit-config.yaml in the demo but that's the only thing doctored, I've figured it would be a nice addition to the README to show how pre-commit works.

@skang0601
Copy link
Collaborator

Thanks! That's super cool to learn about. Then this LGTM, I'll go ahead and merge it in.

@skang0601 skang0601 merged commit 2a8b9cc into norwoodj:master Oct 20, 2021
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.

4 participants