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

Added pre-commit configuration for black & isort. #16

Merged
merged 3 commits into from
Jan 6, 2023

Conversation

apollo13
Copy link
Contributor

@apollo13 apollo13 commented Jan 6, 2023

Hi Peter, I have added support for pre-commit to ensure that the code stays black/isort/flake8 formatted. I basically took the existing configuration from Django. Would you mind approving the workflow so we can see how it looks like?

I have also tightened the permissions of the workflows etc… Let me know if you dislike something.

@ipmb
Copy link
Member

ipmb commented Jan 6, 2023

Want to just go all-in on pyproject.toml and ditch setup.cfg as well?

@apollo13
Copy link
Contributor Author

apollo13 commented Jan 6, 2023

Why not, that would require us to choose a different build backend. Do you have anything in mind?

@apollo13 apollo13 force-pushed the pre-commit branch 2 times, most recently from 4f71dc3 to 80a88d4 Compare January 6, 2023 17:13
@ipmb
Copy link
Member

ipmb commented Jan 6, 2023

I think it's possible to still use setuptools now, but I've also used flit in the past and been happy with it.

@apollo13
Copy link
Contributor Author

apollo13 commented Jan 6, 2023

Ok, I am not sure if we would get rid of setup.cfg completly because some tools might only support that, but let's see

@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2023

Codecov Report

Merging #16 (f323312) into main (71d3641) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              main       #16    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            3         9     +6     
  Lines          157       418   +261     
  Branches        35        48    +13     
==========================================
+ Hits           157       418   +261     
Impacted Files Coverage Δ
goodconf/contrib/argparse.py 100.00% <100.00%> (ø)
tests/test_initial.py 100.00% <0.00%> (ø)
tests/test_files.py 100.00% <0.00%> (ø)
tests/test_file_helpers.py 100.00% <0.00%> (ø)
tests/utils.py 100.00% <0.00%> (ø)
tests/test_django.py 100.00% <0.00%> (ø)
tests/test_goodconf.py 100.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor Author

@apollo13 apollo13 left a comment

Choose a reason for hiding this comment

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

@ipmb Moved most of the stuff into pyproject.toml. I'd ask you to review this very carefully before merging -- there might be dragons.

Programming Language :: Python :: 3.11

[options]
packages = find:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The find stuff is no longer needed, setuptools & setuptools_scm seems to do the correct thing.

[zest.releaser]
tag-signing = yes
date-format = %%-d %%B %%Y
tag-format = v{version}

[check-manifest]
ignore =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped this and opted to include tests in the sdist -- let me know if you have a problem with that?

tests/*

[coverage:run]
source = goodconf
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped because I don't know what was using it, maybe the old way of running coverage required it?

[coverage:report]
show_missing = true
skip_covered = true
[flake8]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flake8 doesn't support pyproject.toml yet, I could only move it into .flake8, but as long as zest stays we can keep setup.cfg?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we can keep setup.cfg for now. Moving to ruff (via flake8-to-ruff might be an option, but that's low priority.

dropping this here in case the situation changes: PyCQA/flake8#234

setup.cfg Show resolved Hide resolved
@apollo13 apollo13 marked this pull request as ready for review January 6, 2023 17:59
@ipmb
Copy link
Member

ipmb commented Jan 6, 2023

I was able to run python -m build and get a tarball and wheel that were installable via pip. lgtm

@ipmb ipmb merged commit 188d1fb into lincolnloop:main Jan 6, 2023
@apollo13 apollo13 deleted the pre-commit branch January 12, 2023 19:35
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.

3 participants