Skip to content

Conversation

@encukou
Copy link
Member

@encukou encukou commented Nov 11, 2025

Without colours, the script's output hard to scan for my spoiled eyes.

The messages are adjusted to no longer imply a lack of exceptions to the rules.

(This PR is not tightly related to #141376 but I think it's best to attach it to that issue.)

@encukou encukou changed the title gh-141391: make smelly: Add colours, adjust messages. gh-141376: make smelly: Add colours, adjust messages. Nov 11, 2025
@encukou
Copy link
Member Author

encukou commented Nov 11, 2025

@AA-Turner @hugovk How can I inform precommit that _colorize is part of the stdandard library?

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Instead of colors, I would prefer to not print each tested binary unless if has "smelly" symbols.

@hugovk
Copy link
Member

hugovk commented Nov 11, 2025

@AA-Turner @hugovk How can I inform precommit that _colorize is part of the stdandard library?

Add this to Tools/build/.ruff.toml:

diff --git a/Tools/build/.ruff.toml b/Tools/build/.ruff.toml
index 996f725fdcb..e7852abe131 100644
--- a/Tools/build/.ruff.toml
+++ b/Tools/build/.ruff.toml
@@ -31,6 +31,9 @@ ignore = [
     "PYI025",  # Use `from collections.abc import Set as AbstractSet`
 ]
 
+[lint.isort]
+extra-standard-library = ["_colorize"]
+
 [lint.per-file-ignores]
 "{check_extension_modules,freeze_modules}.py" = [
     "UP031",  # Use format specifiers instead of percent format

Docs: https://docs.astral.sh/ruff/settings/#lint_isort_extra-standard-library

And note it'll stick it first in the stdlib import list.

First gather data, then present the results.
@encukou
Copy link
Member Author

encukou commented Nov 11, 2025

@hugovk, thanks! Maybe I'll not need it here but hopefully I'll remember next time.

@vstinner, would you be OK with something like https://github.com/encukou/cpython/blob/make-smelly-refactor/Tools/build/smelly.py ?

@vstinner
Copy link
Member

would you be OK with something like https://github.com/encukou/cpython/blob/make-smelly-refactor/Tools/build/smelly.py ?

Yes, something like that 👍

@encukou encukou changed the title gh-141376: make smelly: Add colours, adjust messages. gh-141376: make smelly: List all symbols, repeat smelly ones at end Nov 12, 2025
@encukou
Copy link
Member Author

encukou commented Nov 12, 2025

OK; I've changed the script to instead:

  • List all symbols as they're discovered (which is useful to have in the logs, for checking what the script does)
  • List smelly symbols at the end (so you don't have to dig for them in the verbose output)

This way it's fine without colours!

@encukou
Copy link
Member Author

encukou commented Nov 13, 2025

This version includes removing exceptions (as in #141392).

@vstinner
Copy link
Member

List all symbols as they're discovered (which is useful to have in the logs, for checking what the script does)

Oh. I expected the opposite: only display smelly symbols and be silent if a binary has no smelly symbols.

make smelly|wc -l now returns 3,666 lines. It's way more verbose.

@encukou
Copy link
Member Author

encukou commented Nov 14, 2025

It's more verbose, but that part scrolls up on a terminal; the important part is at the end so it stays visible.
I think it's good to have the full list in logs. But if you disagree strongly I can remove it, of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants