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

Remove lint logic from setup.py #4201

Merged
merged 4 commits into from
Apr 14, 2024
Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Apr 7, 2024

After #4200 is merged remove lint logic from setup.py

To replace the commands that were removed from setup.py, the following will work...

  • pre-commit run black
  • pre-commit run clang-format
  • pre-commit run --all-files

Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

Hi, Thanks for this.

I think we still need a place to put commands that can be run during development. But I'm not sure if there even is a new place in whatever the latest python packaging way is to do this? In old Makefile projects this might be in make check. npm projects have a package.json scripts. Rust projects have task runners like cargo make. Seems tasks aren't standardised in python yet... except in distutils/setuptools commands.

I left a note about the docs check in CI probably still needing to be there. I guess that will move to precommit too eventually?

.github/workflows/format-lint.yml Show resolved Hide resolved
@cclauss cclauss marked this pull request as ready for review April 14, 2024 14:49
@cclauss cclauss changed the title DRAFT: After #4200 is merged remove lint logic from setup.py After #4200 is merged remove lint logic from setup.py Apr 14, 2024
@cclauss
Copy link
Contributor Author

cclauss commented Apr 14, 2024

we still need a place to put commands that can be run during development

The following will work...

  • pre-commit run black
  • pre-commit run clang-format
  • pre-commit run --all-files

setup.py Outdated Show resolved Hide resolved
@cclauss cclauss changed the title After #4200 is merged remove lint logic from setup.py Remove lint logic from setup.py Apr 14, 2024
Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

👍 thanks

@illume illume merged commit 006caf0 into pygame:main Apr 14, 2024
16 of 17 checks passed
@illume
Copy link
Member

illume commented Apr 14, 2024

Not related to this PR exactly...

I noticed the pre-commit that comes with Ubuntu 22.04 is a bit too old for our config. There also aren't any install instructions for pre-commit with Ubuntu and other OS that work for me on the install page https://pre-commit.com/#install
(I installed with pip into my pyenv bin folder, but I guess we will need better pre-commit install instructions for folks on windows/Ubuntu)

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