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

Challenging to contribute PRs because standards are either undocumented or hard to find #2055

Open
jasonwbarnett opened this issue Jul 10, 2024 · 4 comments

Comments

@jasonwbarnett
Copy link
Contributor

jasonwbarnett commented Jul 10, 2024

I was recently contributing a fix in #2053 and ran into a flake8 linting error. I looked at the github workflows to see what was being run. I discovered flake8 is being used to lint, great. What's bad is I didn't know what command to run to automatically format the code to fit the convention. I ended up using black --line-length 79 but that changed a whole host of things that flake8 wasn't complaining about.

So I'm opening this PR in hopes to clarify if there is some automatic way to format/lint code. I'm also opening this up to see if this project has any appetite to bring this project up to the 2024 standards, i.e.

  1. use ruff (abandon flake8)
    • make entire code base compliant with a single standard via ruff check and ruff format
  2. convert project to use a pyproject.toml
  3. Use either poetry or uv

To be clear, I'd be more than willing to help transition to these things. It looks like celery already has some of this.

@jasonwbarnett
Copy link
Contributor Author

jasonwbarnett commented Jul 10, 2024

Maybe this is a discussion? Not sure if a project maintainer can migrate this over or not.

I also dug a little deeper to find out that this project is tightly coupled to flake8 with things like flakeplus. So it seems like migrating to a different linter might be a bigger deal than I originally thought. It also seems like that was created when older versions of Python (i.e. 2.5 and 2.7) were supported which it seems by the project setup.py and github workflows may no longer be the case? If so, can flakeplus be dropped?

I also found the sweet Makefile, so I feel a bit dumb for not seeing that in the first place. I still didn't observe anyway to automatically remediate lint failures in the code.

@jasonwbarnett
Copy link
Contributor Author

I also found the .pre-commit-config.yaml 🤦‍♂️

@Nusnus
Copy link
Member

Nusnus commented Jul 11, 2024

There was a discussion some time ago (year+) about upgrading the linting tools generally (celery & kombu), with our eyes on Ruff.
At the time, we concluded that it wasn’t mature enough, but ever since, I’ve had my eyes closely on Ruff.
As time passed, I also came to know of uv and I’ve actually used Poetry completely in pytest-celery (including full CI/CD).

I’m now actively monitoring every release of Ruff, uv, and Poetry, going through every changelog item, and making sure I’m up to date with the recent changes.
My personal opinion is that indeed Ruff is mature enough but as you said, we are a bit too entangled with the existing toolchain so it will require some effort to upgrade our linting.

Regarding uv/poetry. As pytest-celery uses poetry and generally poetry is very mature, I’d vote for Poetry. That being said, uv and Ruff are of the same family, so it can also go hand-in-hand.

@stumpylog
Copy link

I would personally add another vote for using ruff for linting and formatting. It lowers the bar significantly to new contributors, being just 1 program and two commands to run. Plus it really is that fast, with the option to enable a large number of linting rules

Doing a packaging switch to something like poerty or hatch would be a good opportunity to also update to pyproject.toml instead of the mix of setup.* files

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

3 participants