open-telemetry / opentelemetry-python

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

Use ruff instead of black/isort for autoformatting #3822

Open aabmass opened 6 months ago

aabmass commented 6 months ago

Description

Part 1/2 for https://github.com/open-telemetry/opentelemetry-python/issues/3260

This only migrates the auto formatting capabilities. Linting will be in a separate PR which could use more scrutiny. Most of these changes look formatting changes and small things. See https://docs.astral.sh/ruff/formatter/black/ for known differences.

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

N/A this only reformats the code

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:

xrmx commented 6 months ago

BTW do you see what's wrong with the Public API check?

aabmass commented 6 months ago

BTW do you see what's wrong with the Public API check?

I think it's just flaking, the output doesn't have any results even though its failing

aabmass commented 6 months ago

Actually @xrmx this PR fixes the check https://github.com/open-telemetry/opentelemetry-python/pull/3825

xrmx commented 6 months ago

BTW do you see what's wrong with the Public API check?

I think it's just flaking, the output doesn't have any results even though its failing

I think at least something like this is required:

diff --git a/scripts/public_symbols_checker.py b/scripts/public_symbols_checker.py
index 05b7ad4a..b9802267 100644
--- a/scripts/public_symbols_checker.py
+++ b/scripts/public_symbols_checker.py
@@ -124,10 +124,12 @@ def remove_common_symbols():

 if added_symbols or removed_symbols:
-
     # If a symbol is added and removed in the same commit, we consider it
     # as not added or removed.
     remove_common_symbols()
+
+# re-evaluate symbols after remove_common_symbols side-effects
+if added_symbols or removed_symbols:
     print("The code in this branch adds the following public symbols:")
     print()
     for file_path_, symbols_ in added_symbols.items():

UPDATE: ah seen your PR :)

xrmx commented 6 months ago

BTW changelog would be nice to have the ruff commit handy

aabmass commented 6 months ago

BTW changelog would be nice to have the ruff commit handy

I didn't add an entry initially because I don't think it's applicable to the changelog audience (consumers of the library). Happy to add if you think otherwise

xrmx commented 6 months ago

BTW changelog would be nice to have the ruff commit handy

I didn't add an entry initially because I don't think it's applicable to the changelog audience (consumers of the library). Happy to add if you think otherwise

It would be handy for people poking with the code :)

aabmass commented 6 months ago

Adding "do not merge label" until we decide for sure on Ruff replacing flake8 and pylint as well.

ocelotl commented 6 months ago

Adding "do not merge label" until we decide for sure on Ruff replacing flake8 and pylint as well.

Marking as draft to prevent accidental merging.

xrmx commented 5 months ago

Once the conflict has been resolved this should be ready to land right?