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

PR: Setup pre-commit and format all code with Black #451

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

Czaki
Copy link
Contributor

@Czaki Czaki commented Aug 25, 2023

This PR setup pre commit with following hooks:

  • black - code formatter
  • ruff - statical code linter. Extremely fast, under active development that increases the number of rules. May need to revisit the list of rules to see if my selection (copied from napari) is ok. Here is a list of rules: https://beta.ruff.rs/docs/rules/ (selection is not full, ass we do not revisit it in napari for a long time). Main downside is that when pre-commit.ci makes PR with an update (set to monthly interval), there may be a need to introduce some fixes/blacklist some rules.
  • check-jsonschema check github workflows
  • setup-cfg-fmt - setup.cfg formatter
  • pre-commit/pre-commit-hooks - set of simple check for common simple mistakes like mixed line endings, validation json, yaml and toml for proper syntax (but not schema).

closes #345

@Czaki
Copy link
Contributor Author

Czaki commented Aug 25, 2023

@dalthviz I think that best option for smooth experience is to setup pre-commit.ci on this repository.
Thom napari experience it provides nice autofix features if possible.

@Czaki Czaki marked this pull request as draft August 25, 2023 18:35
@dalthviz
Copy link
Member

dalthviz commented Aug 25, 2023

Thank you so much for the help here @Czaki ! I think using pre-commit.ci makes sense 👍 I believe, probably after this get merged, that to enable it we just need to follow the web setup, right?

@dalthviz dalthviz added this to the v2.4.0 milestone Aug 25, 2023
@ccordoba12
Copy link
Member

ccordoba12 commented Aug 25, 2023

Thanks for giving us a hand with this @Czaki!

One important thing to mention is that I think all changes in this PR should be squashed in a single commit and when everything is ready for merge, that commit should be added to .git-blame-ignore-revs, so we can still rely on git blame after this.

@Czaki Czaki marked this pull request as ready for review August 25, 2023 22:23
@Czaki
Copy link
Contributor Author

Czaki commented Aug 25, 2023

Squased. I hope that all imports that I removed by mistake are found.

@ccordoba12 ccordoba12 changed the title setup pre-commit PR: Setup pre-commit and format all code with Black Aug 25, 2023
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the work @Czaki ! I left some comments regarding the new contribution guide pre-commit entry, and some code comments which seems like either need to be removed now or due to imports changing order need to be moved along side the new place the imports is.

Besides that I noticed a removed PySide6 import inside a test that maybe could be worthy to check locally but other than that this LGTM 👍 Again thank you so much for helping us with this!

CONTRIBUTING.md Outdated Show resolved Hide resolved
qtpy/QtGui.py Outdated Show resolved Hide resolved
qtpy/__init__.py Outdated Show resolved Hide resolved
qtpy/__init__.py Outdated Show resolved Hide resolved
qtpy/tests/conftest.py Show resolved Hide resolved
qtpy/tests/test_main.py Show resolved Hide resolved
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Czaki !

@dalthviz dalthviz merged commit 354461c into spyder-ide:master Aug 28, 2023
12 checks passed
@Czaki Czaki deleted the pre-commit-setup-ci branch August 28, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use pre-commit to automatically format code with Black and imports with isort
3 participants