-
Notifications
You must be signed in to change notification settings - Fork 53
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
Issue with pre-commit hooks and src structure #214
Comments
I agree, I don't think that's the right fix. The error you're encountering is because More generally, I had a quick look locally and I couldn't reproduce the problem, but I am a bit unfamiliar with the development workflow for pre-commit hooks. I agree though it would be good to make the experience nicer, so happy to consider a pull request that sorts this out. |
I think I figured it out, |
The way pre-commit is designed, it will try to create a managed virtual environment to run the tool. But these venvs are not configurable - e.g. you can't run I think the best solution for The alternative option, in #216 is to use the system Python to run the tool. But this has its own drawbacks (for example it is not compatible with |
I've added
import-linter
to mypre-commit
hooks for my copywriter project. I use asrc/
folder structure where my source code is insrc/copywriter
and my tests are intests/
in the root directory.My
.import-linter
file looks like this:and my
pre-commit
setup looks like this:I've noticed that even though running
lint-imports
directly works fine, running it viapre-commit
returned the following error:Could not find package 'copywriter' in your Python path.
. I figured it had to do with thesrc
structure as if I move mycopywriter
folder to the top directory, the error disappears.I am not sure if it's a bug or a feature, so to me, these are the options:
src/
folder.src/
folder,.import-linter
or.pre-commit-config.yaml
need to be configured differently.If it's the second, I am unsure of the preferred way. I've turned
copywriter
intosrc.copywriter
in my.import-linter
file, and it seems to have fixed the problem, but I'm not a fan.If we resolve this issue, it would be wise to add something in the documentation, see my other issue.
The text was updated successfully, but these errors were encountered: