Skip to content

[feature] add hooks for using with pre-commit#157

Merged
fpgmaas merged 1 commit intofpgmaas:mainfrom
rrauenza:feature
Oct 23, 2022
Merged

[feature] add hooks for using with pre-commit#157
fpgmaas merged 1 commit intofpgmaas:mainfrom
rrauenza:feature

Conversation

@rrauenza
Copy link
Contributor

@rrauenza rrauenza commented Oct 7, 2022

PR Checklist

  • A description of the changes is added to the description of this PR.
  • If there is a related issue, make sure it is linked to this PR.
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

Description of changes

Second try at this based on previous reviews -- do we still want to try to provide a hooks file?

@wpdonders

Reference: #124

@fpgmaas fpgmaas self-requested a review October 8, 2022 05:38
- "--skip-missing"
```

Replace <tag> with one of the [tags](https://github.com/fpgmaas/deptry/tags) from the
Copy link
Owner

Choose a reason for hiding this comment

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

Suggestion to replace the NOTE: with an admonition using the !!! info syntax.

Also, maybe we should state more explicitly that the pre-commit hook should be run within the virtual environment, and we could replace the word virtualenv with virtual environment.

!!! important

    This will only pull in the pre commit hooks config file from the version you have specified.  The actual version of deptry that will be run will be the first one found in your path, so you will need to add deptry to your local virtual environment for the pre-commit.
    
    For the pre-commit hook to run successfully, it should be run within the virtual environment of the project to be scanned, since it needs access to the metadata of the installed packages.
   

@fpgmaas
Copy link
Owner

fpgmaas commented Oct 8, 2022

Looks good to me, thanks for your contribution. I tested it out locally on another repository and it seems to work great!

I added one comment about the documentation.

@fpgmaas
Copy link
Owner

fpgmaas commented Oct 13, 2022

@rrauenza

@rrauenza rrauenza force-pushed the feature branch 2 times, most recently from 0e5d91b to 4052ece Compare October 13, 2022 20:48
@rrauenza
Copy link
Contributor Author

@fpgmaas ,

Will github render the !!! info ...? I'm not seeing it rendered here (and "important" doesn't seem to be a valid tag?):

https://github.com/rrauenza/deptry/blob/feature/docs/docs/usage.md

@fpgmaas
Copy link
Owner

fpgmaas commented Oct 13, 2022

@fpgmaas ,

Will github render the !!! info ...? I'm not seeing it rendered here (and "important" doesn't seem to be a valid tag?):

https://github.com/rrauenza/deptry/blob/feature/docs/docs/usage.md

@rrauenza Good that you check. This file is used by mkdocs to generate the documentation for deptry. The syntax you see there is an admonition; https://squidfunk.github.io/mkdocs-material/reference/admonitions/

So it's no problem you don't see it being rendered properly on Github. Hope that clarifies it!

@fpgmaas fpgmaas merged commit d295256 into fpgmaas:main Oct 23, 2022
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.

2 participants