Replace pre-commit by prek #2533
Conversation
luizvbo
force-pushed
the
replace-pre-commit-by-prek
branch
from
80b6728 to
2346c2b
Compare
luizvbo
force-pushed
the
replace-pre-commit-by-prek
branch
from
2346c2b to
256a370
Compare
|
Closes #2533 |
|
Hello! Could you review this PR for me? Thanks in advance |
|
Thanks for working on this! 🥳
This is not really an issue; however, running it from different machines (and also Dependabot does this) will result in more conflicts. |
|
Strange... The tests are working in my machine (python 3.12), but failing in the CI (python 3.11). It's strange, because I didn't touch src. I think it might be caused by updating the packages in the poetry.lock file |
|
@Fokko, I think the issue is fixed now (tests are passing locally with py 3.11). I had to set an upper limit for the datafusion package, given that tests fail in py3.11 after version 48. |
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Nice, this looks great @luizvbo! I'm positive, let's see what @kevinjqliu thinks!
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
LGTM
An improvement in runtime!
prek: make lint 8.10s user 1.18s system 255% cpu 3.638 total
poetry: make lint 11.65s user 1.76s system 171% cpu 7.815 total
| thrift-sasl = { version = ">=0.4.3", optional = true } | ||
| kerberos = {version = "^1.3.1", optional = true} | ||
| datafusion = { version = ">=45", optional = true } | ||
| datafusion = { version = ">=45,<49", optional = true } |
There was a problem hiding this comment.
i tested locally that this passes without the extra <49
poetry lock
make install
poetry run pytest tests/table/test_datafusion.py
There was a problem hiding this comment.
its fine to leave as is. due to limitations we actually cannot use datafusion 49, so this makes sense
3 participants
Rationale for this change
It replaces pre-commit with prek, which is almost 10x faster. prek is a port of pre-commit to rust.
Are these changes tested?
Yes.
make lintis running locallyAre there any user-facing changes?
Only for developers, given that it affects
make lint,NOTE: I had to update the poetry.lock file with
poetry lock, resulting in multiple changes. Maybe it's better to run again in your machine (I don't know if it's run automatically in the CI pipeline).