Comments (9)
I'm strongly opposed to this.
I've used system commands very deliberately when I added pre-commit to this project. That's because not only do some linters not have official pre-commit hooks provided (even though that might no longer be the case for those which we're using now, it might become relevant the moment we try to move to something else, and I don't want to have to have some local hooks, and other CI hooks), but even the tools which do provide these often don't do a great job at keeping them up-to-date with the latest actual release of that tool.
The version of the individual tools used by pre-commit locally are all listed in pyproject.toml
, and it is your responsibility to run poetry install
to install them locally, and to update the dependencies as pyproject.toml
changes. This is the case for any project dependencies, not just those used in pre-commit.
It is not enough to simply declare the dependencies in pre-commit, and to omit these tools from pyproject.toml entirely, as we need the actual executables to be made available in the project's virtual environment. This is because many editors pick up on that, and then use the proper versions of these linters, as installed in the environment.
I'm also against the option to have the versions listed both in pre-commit, and in pyproject.toml, as we would need to keep those versions consistent, and pay great attention that this remains the case as updates come out.
In my opinion, there is no reason to oppose using locally installed linters, considering they're properly declared and will be installed with poetry install
, and if you just have an issue with running poetry install
every time the pyproject.toml
is updated, perhaps then it may be worth it to create a script that runs as a part of pre-commit (another local pre-commit hook), and checks that your version of these tools matches the one declared in pyproject, though I'm not sure how hard of a task this would be. Alternatively, we could even add poetry install
as a command to just run as a part of pre-commit, before any of the other linters run.
from mcstatus.
Revoked pre-commit's access to this repo until this issue will be fixed/resolved (I mean, there is no sense to keep CI turned on if it always fails)
from mcstatus.
Doesn't it mean that we have to install one dependency twice?
from mcstatus.
Not if you only run pre-commit, no. If you run the full CI locally, then yes, this would be 'installing it twice', and there's no real way to access pre-commit's virtual environments manually in a nice fashion I'm pretty sure. I think it would be worth it though, because I don't like the idea of what's running being system dependent. I would much rather have pre-commit handle installing the correct version of everything than having the system have the right version of everything, if that makes any sense.
from mcstatus.
And by the way, we already run pre-commit in Validation workflow
though I'm not sure how hard of a task this would be
https://python-poetry.org/docs/master/pre-commit-hooks/#poetry-install
from mcstatus.
https://python-poetry.org/docs/master/pre-commit-hooks/#poetry-install
Oh, that's really nice! This would be a skipped hook in the GitHub CI, as we install with ItsDrike/setup-poetry action, but for local updating, I think it might actually be worth doing. poetry install
is a no-op if there were no updates anyway, so it's pretty much perfect.
What I do wonder about here is whether using the official hook would also install poetry as a tool, it it's not found. I very much doubt it, but if it would, we could actually use that to fix pre-commit.ci
from mcstatus.
You can add a skip=[<hook id>,<hook id>]
in in the ci
section if required
from mcstatus.
You can add a
skip=[<hook id>,<hook id>]
in in theci
section if required
Can we just skip all of the local hooks then? This would probably solve the issue of it failing, without having to change to the remote hooks, no?
So, unless there's a reason this wouldn't work, or a really good reason to convince me against local hooks that addresses the points I made earlier, I'd like to see this solution instead.
from mcstatus.
Pre-commit.ci was briefly adopted to update tag versions in pre-commit-config.yaml. This is something that pre-commit autoupdate
can do. This issue emerged as part of adopting pre-commit.ci, which was already abandoned as an effort. Moving to close this.
from mcstatus.
Related Issues (20)
- IndexError: bytearray index out of range HOT 28
- ImportError HOT 3
- README.md documentation link open blank docs page HOT 10
- Query throws a "OSError: [Errno 107] Transport endpoint is not connected" HOT 24
- Docs? HOT 3
- [Errno 57] Socket is not connected HOT 15
- Not possible to access RawJavaResponse for type annotations HOT 2
- Parse forge mods from status response HOT 13
- What is this? Is this a mod? HOT 4
- Transport Endpoint is not connected when run on linux HOT 20
- TimeoutError: timed out HOT 18
- `JavaServer.lookup`: Uncaught exception `dns.resolver.LifetimeTimeout` HOT 5
- Setup issue templates on GitHub that include a read-these-common-things message to cut down on the number of Invalid issues HOT 1
- Document possible exceptions HOT 1
- Document windows asyncio event loop close bug
- Enable pre-commit CI auto-updating.
- Support mypy HOT 8
- Automatically approve dependabot PRs HOT 5
- Unable to get status from recent version of Velocity server HOT 6
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mcstatus.