feat: add pre-commit hook support #795
Conversation
| additional_dependencies: | ||
| - github.com/vektra/mockery/v2@latest | ||
| entry: mockery |
There was a problem hiding this comment.
Rather than using additional_dependencies, the pre-commit-hook will already have access to the file system. We should just be able to use go run . to enact the mockery ... command I believe.
Also, we may only want this hook to run when .go files are in the commit. What do you think?
| additional_dependencies: | |
| - github.com/vektra/mockery/v2@latest | |
| entry: mockery | |
| entry: go run . | |
| files: \.go$ |
There was a problem hiding this comment.
I'm unsure it will launch the code you expect to run.
I will make some tests.
But, I think keeping mockery call here would be clearer.
There was a problem hiding this comment.
Using additional dependencies like this won't allow people to specify version in pre commit file because no matter what version they reference, you're pulling latest
There was a problem hiding this comment.
Indeed, I'm unsure if there is something that can be done here, I will have to check
There was a problem hiding this comment.
For now,it seems to me, that it would imply to maintain the version in sync in this file vs the tag. Quite a pain
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| repos: | |||
| - repo: https://github.com/ccoVeille/mockery | |||
There was a problem hiding this comment.
Is this just in here for testing right now? Unless mockery uses itself to generate mocks, and wants to use the pre-commit functionality, I don't think this is required to get this to work for consumers.
There was a problem hiding this comment.
Yes, only for the test. I forgot to mention about it in the PR
I had to create a fake "2.43.3" tag on my repository to be able to make it work.
|
|
||
| ```yaml | ||
| repos: | ||
| - repo: https://github.com/ccoVeille/mockery |
There was a problem hiding this comment.
| - repo: https://github.com/ccoVeille/mockery | |
| - repo: https://github.com/vektra/mockery |
There was a problem hiding this comment.
I kept it in sync to help mockery mainteners to be able to test
But yes, I will have to change this one
| ```yaml | ||
| repos: | ||
| - repo: https://github.com/ccoVeille/mockery | ||
| rev: v2 |
There was a problem hiding this comment.
nit/nb: It would be great if this could be automatically updated on new releases.
| rev: v2 | |
| rev: v2.43.2 |
There was a problem hiding this comment.
There is no need usually. precommit manage this well with this kind of tag
| Then install the hook with the following command: | ||
|
|
||
| ```shell | ||
| pre-commit autoupdate |
There was a problem hiding this comment.
Ah, this is why you have it as v2 above, calling for autoupdate will which bump the version to latest. May be better to be explicit about the versioning system, and call for pre-commit install versus autoupdate.
There was a problem hiding this comment.
Indeed, I'm used to use autoupdate for everything.
I think "pre-commit install" would also bump it locally. I will make some tests.
Then I will update the documentation based on my tests
| @@ -0,0 +1,6 @@ | |||
| --- | |||
There was a problem hiding this comment.
Note for myself
I will have to add Co-Authored-By metadata
|
anyway @liveFreeOrCode could you confirm it "works" as this ? |
|
@liveFreeOrCode (or anyone) could you confirm ? thanks Then I will remove the oddities present to make it works with my branch, and switch to vektra/mokery one |
|
You commented the PR, thanks. But you didn't reply my question. Does it work on your computer now with current code I provided. I understand you might dislike the current implementation. I agree it could be improved. But right now,I would like to get a simple reply to the following question. Does it work? |
Yeah sorry I'm traveling without my PC. I can't test until next week |
|
OK thanks for replying, we can also wait |
Description
Add pre-commit.com support in mockery.
Type of change
Version of Go used when building/testing:
How Has This Been Tested?
Locally, by following the guide I provide in installation.md file
Checklist