celery / kombu

Messaging library for Python.
http://kombu.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
2.85k stars 926 forks source link

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

Open jasonwbarnett opened 1 month ago

jasonwbarnett commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

Nusnus commented 1 month ago

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 commented 1 month ago

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