open-telemetry / opentelemetry-python

OpenTelemetry Python API and SDK
https://opentelemetry.io
Apache License 2.0
1.67k stars 571 forks source link

Migrate formatting and linting to ruff #3816

Closed aabmass closed 3 months ago

aabmass commented 3 months ago

Description

Migrates pylint, flake8, black, and isort to ruff. Ruff is very fast and has complete feature parity with all of the above except some pylint features. Ruff does not use the python environment at all, so it no longer requires any deps to be installed in the virtualenv.

Dropping pylint is definitely worth discusing, because we don't run mypy on all of our sources. See here for a list of tradeoffs and recommendations. I think this is a decent trade off given how clunky, difficult to use (e.g. #3814), and slow pylint is. We should try to enable mypy on the rest of our sources imo.

Fixes https://github.com/open-telemetry/opentelemetry-python/issues/3260 and numerous complaints about the painfully slow lint/formatting process

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Does This PR Require a Contrib Repo Change?

Answer the following question based on these examples of changes that would require a Contrib Repo Change:

Checklist:

aabmass commented 3 months ago

I was going to do it after bumping black but thanks for beating me to it :)

Sorry didn't realize you were working on it! I can give you permission to push to the PR if you'd like to collaborate here

Fyi I should probably split this into two PRs (or at least commits) for easier reviewing:

  1. replace black and isort
  2. replace linters and swap any pylint-disable with ruff noqa comments

I think the second will be more useful to actually scrutinize for missing coverage

xrmx commented 3 months ago

I was going to do it after bumping black but thanks for beating me to it :)

Sorry didn't realize you were working on it! I can give you permission to push to the PR if you'd like to collaborate here

Don't worry, it was just a plan :)

Fyi I should probably split this into two PRs (or at least commits) for easier reviewing:

1. replace black and isort

2. replace linters and swap any `pylint-disable` with ruff `noqa` comments

I think the second will be more useful to actually scrutinize for missing coverage

Separate commits for formatting and linting will be appreciated, thanks!

aabmass commented 3 months ago

Closing in favor of two PRs. First one here https://github.com/open-telemetry/opentelemetry-python/pull/3822