Skip to content
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

pre-commit and pre-commit-ci #1672

Merged
merged 4 commits into from Feb 6, 2022

Conversation

aucampia
Copy link
Member

@aucampia aucampia commented Jan 16, 2022

Add pre-commit and pre-commit-ci with the following hooks:

  • black for formatting code.

For more info see docs/developers.rst.

@aucampia
Copy link
Member Author

@eggplants Please have a look if you have a moment and let me know if you have any feedback.

@aucampia
Copy link
Member Author

For a demo see: aucampia#33

I have enabled pre-commit CI now, but until this is merged it will always fail, it can be easily disabled from here: https://pre-commit.ci/

If we are in agreement on the settings there are the following options to actually run the hooks on the codebase:

  1. We merge it and then wait for the weekly pre-commit CI auto fix - this happens 18:00 UTC Monday
  2. We merge it and then make a emptyish PR and run pre-commit.ci autofix on it

Once this is done things will be fairly clean going forward, and each contributor can then either run pre-commit run by themselves or they/us can run pre-commit.ci autofix on their PR branches.

@aucampia aucampia force-pushed the iwana-20220114T2135-precommit_ci branch from dd82991 to 039096d Compare January 16, 2022 13:16
@aucampia
Copy link
Member Author

aucampia commented Jan 16, 2022

Pycln is fairly conservative on what it cleans, it does actually end up leaving a bunch of unused imports, but only because they have side effects, for more info see: https://hadialqattan.github.io/pycln/#/?id=side-effects

@aucampia aucampia force-pushed the iwana-20220114T2135-precommit_ci branch 5 times, most recently from eaa5a5e to 515e605 Compare January 16, 2022 14:11
@aucampia
Copy link
Member Author

I will disable pre-commit.ci until this is merged. Can be controlled using github IAM at https://results.pre-commit.ci/ for future reference.

@aucampia aucampia force-pushed the iwana-20220114T2135-precommit_ci branch from 7aeefb2 to e6aaa19 Compare January 22, 2022 19:33
@aucampia
Copy link
Member Author

Rebased to master branch.

I messed up the old demo, new one here: aucampia#35

@aucampia aucampia mentioned this pull request Jan 24, 2022
@aucampia
Copy link
Member Author

Will split the tox changes off this also as those will actually make development quite a bit easier.

@aucampia
Copy link
Member Author

Given some feedback on other issues I'm going to scale this back to only run black to begin with, we can add other things afterwards.

@aucampia aucampia marked this pull request as draft January 24, 2022 08:02
@aucampia aucampia mentioned this pull request Jan 24, 2022
@aucampia
Copy link
Member Author

tox changes split into #1691

@aucampia aucampia force-pushed the iwana-20220114T2135-precommit_ci branch from 6f7d0d9 to 2ac7771 Compare January 24, 2022 22:16
@aucampia
Copy link
Member Author

Scaled this down to only do black.

@aucampia aucampia marked this pull request as ready for review January 24, 2022 22:45
Add pre-commit and pre-commit-ci with the following hooks:

- black for formatting code.

For more info see `docs/developers.rst`.
@aucampia aucampia force-pushed the iwana-20220114T2135-precommit_ci branch from 2ac7771 to b2e124d Compare January 29, 2022 23:59
Can be added later with the rest of isort.
This is to work around this issue: psf/black#2493

Without doing this the pre-commit hook for black fails as it thinks it
got the wrong version.
@aucampia aucampia force-pushed the iwana-20220114T2135-precommit_ci branch from fadf20a to 3700b6c Compare January 30, 2022 14:52
@aucampia
Copy link
Member Author

I changed the hook slightly to work with the version spec in black config, for more info see psf/black#2493.

Demo updated and re-run: aucampia#35

Will merge next week sometime.

@aucampia aucampia merged commit 2cab8a3 into RDFLib:master Feb 6, 2022
@aucampia aucampia deleted the iwana-20220114T2135-precommit_ci branch April 9, 2022 14:14
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.

None yet

2 participants