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.

Sign up for GitHub

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 hook support #795

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

feat: add pre-commit hook support #795

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df6b998
feat: add pre-commit hook support
ccoVeille Jul 17, 2024
1d56404
Merge branch 'master' into pre-commit
ccoVeille Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for myself

I will have to add Co-Authored-By metadata

repos:
- repo: https://github.com/ccoVeille/mockery
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@ccoVeille ccoVeille Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

rev: v2
hooks:
- id: generate-mocks
9 changes: 9 additions & 0 deletions .pre-commit-hooks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
- id: generate-mocks
name: generate mocks
description: Generate mocks with mockery
language: golang
additional_dependencies:
- github.com/vektra/mockery/v2@latest
entry: mockery
Comment on lines +6 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Suggested change
additional_dependencies:
- github.com/vektra/mockery/v2@latest
entry: mockery
entry: go run .
files: \.go$

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I'm unsure if there is something that can be done here, I will have to check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible let the users assume the responsibility for installing mockery? If that's the case this should suffice

---
- id: generate-mocks
  name: generate mocks
  description: Generate mocks with mockery
  language: golang
  entry: mockery
  pass_filenames: false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, of course, it makes sense.

But then having 2 rules could be a something to consider

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your approach is the most commonly used in fact

https://github.com/search?q=path%3A%22%2F.pre-commit-hooks.yaml%22+golang&type=code

pass_filenames: false
26 changes: 26 additions & 0 deletions docs/installation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,32 @@ Generate all the mocks for your project:

docker run -v "$PWD":/src -w /src vektra/mockery --all

### pre-commit

Add the following lines to your `.pre-commit-config.yaml` file

```yaml
repos:
- repo: https://github.com/ccoVeille/mockery
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- repo: https://github.com/ccoVeille/mockery
- repo: https://github.com/vektra/mockery

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it in sync to help mockery mainteners to be able to test

But yes, I will have to change this one

rev: v2
ccoVeille marked this conversation as resolved.
Show resolved Hide resolved
hooks:
- id: generate-mocks
```

!!! note

If the file is missing, you can simply create it with this content

Then install the hook with the following command:

```shell
pre-commit autoupdate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

```

It will install everything. And from now, your commit will trigger mock generation.

More information about pre-commit can be found on [pre-commit.com/](https://pre-commit.com/)

### Homebrew

Install through [brew](https://brew.sh/)
Expand Down
Loading