RDFLib / sparqlwrapper

A wrapper for a remote SPARQL endpoint
https://sparqlwrapper.readthedocs.io/
Other
513 stars 121 forks source link

pre-commit #195

Open eggplants opened 2 years ago

eggplants commented 2 years ago

https://github.com/RDFLib/sparqlwrapper/pull/184#discussion_r780823349

I think what is needed:

eggplants commented 2 years ago

@aucampia Are you going to prepare for pre-commit CI?

nicholascar commented 2 years ago

Yes, I think we are fine with pre-commit. Can you please use exactly the same black & flake8 settings are rdflib though? You will see in there that there is a fixed parameters, version-locked black as well as a Flak8 with a few settings specified.

aucampia commented 2 years ago

@eggplants I made a PR for adding pre-commit to https://github.com/RDFLib/rdflib and enabled pre-commit CI for it.

If you make a PR for adding pre-commit config here here I will enable pre-commit CI for this repo also.

Regarding flake8, we still need to rationalize our config for https://github.com/RDFLib/rdflib - some work was started on this in https://github.com/RDFLib/rdflib/pull/1442 but we have to finish it. So for now I'm not adding it because it will always fail. You can add it but please try and keep PRs small to make it easier to review, enabling all checks for flake8 at once and fixing all their issues at once is maybe going too far, but we will do our best with reviews.

The regarding mypy, I did not add it to pre-commit as we run this as part of tox and our CI, I'm open to other views here, but since tests are separate anyway I don't see much benefit to having mypy in pre-commmit as opposed to in tox.

aucampia commented 2 years ago

For reference, PR on RDFLib/rdflib is:

aucampia commented 2 years ago

One of the main benefits of pre-commit.ci for me is the ability to do pre-commit.ci autofix in PRs, so what I put in there is mainly geared towards things that can be automatically fixed, like isort, pycln and black.

eggplants commented 2 years ago

@aucampia Could you install pre-commit in sparqlwrapper as well as rdflib?