Skip to content

Conversation

pdgendt
Copy link
Contributor

@pdgendt pdgendt commented Oct 16, 2025

Partially revert 1bcc065

This change impacts all other scripts that import those modules, while those weren't updated.

It will be hard to maintain this list, whilst keeping the entire tree compliant.

Running the linter on the tree:

# before
$ ruff check .
Found 54 errors.
# after
$ ruff check .
All checks passed!

Partially revert 1bcc065

This change impacts all other scripts that import those modules, while
those weren't updated.

It will be hard to maintain this list, whilst keeping the entire tree
compliant.

Signed-off-by: Pieter De Gendt <[email protected]>
Copy link

@fundakol
Copy link
Contributor

@pdgendt Does it fails on CI or just when you run ruff locally? I haven't seen any issues when I ran check_compliance.py. Does check_compliance.py script run ruff in different way?

@pdgendt
Copy link
Contributor Author

pdgendt commented Oct 16, 2025

@pdgendt Does it fails on CI or just when you run ruff locally? I haven't seen any issues when I ran check_compliance.py. Does check_compliance.py script run ruff in different way?

It didn't fail in CI because it only checks for changed files in a PR (will post updated check soon ASAP)

EDIT: see #97700

@fundakol
Copy link
Contributor

@pdgendt Does it fails on CI or just when you run ruff locally? I haven't seen any issues when I ran check_compliance.py. Does check_compliance.py script run ruff in different way?

It didn't fail in CI because it only checks for changed files in a PR (will post updated check soon ASAP)

EDIT: see #97700

Wouldn't it be easier to just fix those files, it's mostly just one blank line in imports?

@pdgendt
Copy link
Contributor Author

pdgendt commented Oct 16, 2025

Wouldn't it be easier to just fix those files, it's mostly just one blank line in imports?

But the list you have added isn't complete, and it would be difficult to maintain such a specific list IMO.
See https://github.com/zephyrproject-rtos/zephyr/actions/runs/18557451733/job/52898227886?pr=97700 for all currently failing files.

What do you think is the added value for having known first-party modules? Other than a separate import block?

@kartben
Copy link
Contributor

kartben commented Oct 16, 2025

@pdgendt Does it fails on CI or just when you run ruff locally? I haven't seen any issues when I ran check_compliance.py. Does check_compliance.py script run ruff in different way?

It didn't fail in CI because it only checks for changed files in a PR (will post updated check soon ASAP)
EDIT: see #97700

Wouldn't it be easier to just fix those files, it's mostly just one blank line in imports?

The main challenge is that the list of "known-first-party" is very incomplete, and very hard / impossible to maintain.
See e.g. how this fails due to things like list_shields or list_hardware not being "allowlisted" in the known-first-party list

@fundakol
Copy link
Contributor

What do you think is the added value for having known first-party modules? Other than a separate import block?

One would know if a module/package is part of zephyr repo or it is third party package. But I understand it is hard to maintain due to technical debt and mess with python scripts in Zephyr.

@pdgendt pdgendt added the Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. label Oct 16, 2025
@jhedberg jhedberg merged commit aad2408 into zephyrproject-rtos:main Oct 16, 2025
45 checks passed
@pdgendt pdgendt deleted the isort-revert-known-first-party branch October 16, 2025 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Linters area: Twister Twister Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants