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

yapf -> black #85

Closed
chrisjsewell opened this issue Dec 7, 2021 · 6 comments
Closed

yapf -> black #85

chrisjsewell opened this issue Dec 7, 2021 · 6 comments

Comments

@chrisjsewell
Copy link
Member

black rules, yapf drools

@ltalirz
Copy link
Member

ltalirz commented Dec 7, 2021

feel free to change, I have no strong feelings about this

@chrisjsewell
Copy link
Member Author

Plus adding some more pre-commit hooks like in https://github.com/aiidateam/aiida-firecrest/blob/main/.pre-commit-config.yaml

@chrisjsewell
Copy link
Member Author

it would also be ideal not to have to use pylint with language: system, which does not play nicely with pre-commit.ci an github desktop, etc. theres some talk of how to sync dependencies here, but unfortunately no current, complete soltuion: pre-commit/pre-commit#945

@ltalirz
Copy link
Member

ltalirz commented Dec 7, 2021

I recently noticed a CI autoupdate PR somewhere (maybe on this plugin, don't remember) but the checks were failing.
I think having to deal with another monthly PR that updates dependencies for each plugin may be more hassle than adding value.

But we can certainly add some other linters and hooks from the example you linked that we feel are useful.

As for the language: system I agree it's not ideal but until there is an all-around better solution I would wait to switch.

@chrisjsewell
Copy link
Member Author

I recently noticed a CI autoupdate PR somewhere
I think having to deal with another monthly PR that updates dependencies for each plugin may be more hassle than adding value.

Oh I very much disagree, I've already added https://pre-commit.ci/ to aiida-core and other aiidateam packages, and we use it in basically every EBP package.
Its great, because otherwise these hook versions gradually get very out of date, and it usually only takes a few seconds to merge

You can also configure the autoupdate frequency etc: https://pre-commit.ci/#configuration

@ltalirz
Copy link
Member

ltalirz commented Jan 18, 2022

done in #88

@ltalirz ltalirz closed this as completed Jan 18, 2022
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

No branches or pull requests

2 participants