Do not exclude build directory from ruff check#43572
Open
arkq wants to merge 7 commits intoproject-chip:masterfrom
Open
Do not exclude build directory from ruff check#43572arkq wants to merge 7 commits intoproject-chip:masterfrom
arkq wants to merge 7 commits intoproject-chip:masterfrom
Conversation
The `build` name is a very generic name which appears in our Python modules and also is a directory for built artifacts. Instead of excluding a generic name (which will disable ruff check in some part of our code base) exclude specific paths.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves code quality by refining the ruff linter configuration. It replaces a broad build directory exclusion with more specific paths, ensuring that Python modules named build are correctly linted. The introduction of new ruff rules (PLE for Pylint errors and RSE for flake8-raise) has driven a series of beneficial code changes across the repository. These changes include adopting more idiomatic Python constructs (like any() and set comprehensions), simplifying exception handling with contextlib.suppress, standardizing logging calls, and cleaning up code by removing unused imports and unnecessary parentheses in raise statements. The changes are well-executed and contribute to better code consistency and maintainability.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the repo’s Python linting configuration so ruff no longer excludes every directory named build, and tightens linting by enabling additional rules. To keep CI green under the expanded lint scope, the PR also applies targeted cleanups across build scripts and a few Python modules (notably logging usage and exception-raising style), and adds unit coverage for the build glob matcher.
Changes:
- Update
pyproject.tomlruff excludes to remove the genericbuildexclusion, replacing it with specific build-artifact paths; enablePLEandRSErule groups. - Refactor/clean Python code to satisfy newly-applied lint rules (module-level loggers, simplified
raise ...). - Add/expand build-script structure and tests (new build targets module, new Genio builder, glob matcher unit test).
Reviewed changes
Copilot reviewed 28 out of 31 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/controller/python/tests/scripts/cluster_objects.py | Simplifies raise statements to comply with RSE. |
| src/controller/python/matter/crypto/p256keypair.py | Simplifies NotImplementedError raises for RSE. |
| src/controller/python/matter/commissioning/init.py | Simplifies NotImplementedError raise for RSE. |
| src/controller/python/matter/clusters/ClusterObjects.py | Simplifies NotImplementedError raises for RSE in cluster object base APIs. |
| scripts/py_matter_idl/matter/idl/lint/type_definitions.py | Simplifies MissingIdlError raising for RSE. |
| scripts/py_matter_idl/matter/idl/generators/storage.py | Simplifies NotImplementedError raises for RSE in generator storage interface. |
| scripts/flashing/psoc6_firmware_utils.py | Simplifies NotImplementedError raises for RSE. |
| scripts/flashing/firmware_utils.py | Simplifies NotImplementedError raise for RSE. |
| scripts/build/test_glob_matcher.py | Adds unit tests for GlobMatcher. |
| scripts/build/test.py | Materializes iterators with list(...) (minor simplification/clarity). |
| scripts/build/runner/shell.py | Switches to module logger usage; adjusts log emission to satisfy logging lint rules. |
| scripts/build/clang_coverage_wrapper.py | Switches to module logger usage and minor simplifications in Click option config. |
| scripts/build/builders/tizen.py | Uses contextlib.suppress and module logger to satisfy linting requirements. |
| scripts/build/builders/telink.py | Uses module logger instead of root logger calls. |
| scripts/build/builders/nxp.py | Uses module logger instead of root logger calls. |
| scripts/build/builders/nuttx.py | Uses module logger instead of root logger calls. |
| scripts/build/builders/nrf.py | Uses module logger instead of root logger calls. |
| scripts/build/builders/genio.py | Adds a new Genio GN-based builder and app enum. |
| scripts/build/builders/esp32.py | Uses module logger instead of root logger calls. |
| scripts/build/builders/efr32.py | Uses module logger instead of root logger calls. |
| scripts/build/builders/builder.py | Uses module logger; simplifies abstract NotImplementedError raises for RSE. |
| scripts/build/builders/bouffalolab.py | Uses module logger and minor boolean simplification. |
| scripts/build/builders/android.py | Uses module logger and minor iteration simplifications. |
| scripts/build/build_examples.py | Removes unused helper and simplifies target lowercasing comprehension. |
| scripts/build/build_darwin_framework.py | Uses contextlib.suppress for simpler exception handling. |
| scripts/build/build/targets.py | Introduces consolidated build target definitions (including Genio). |
| scripts/build/build/target.py | Uses module logger and small simplifications (any(...), generator usage). |
| scripts/build/build/init.py | Uses module logger and removes unused imports. |
| pyproject.toml | Adjusts ruff excludes to avoid generic build exclusion; enables PLE and RSE. |
| credentials/generate_revocation_set.py | Fixes incorrect log.error usage by using proper formatting parameters. |
| build/config/linux/pkg-config.py | Simplifies regexp matching with any(...). |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Python linting so ruff no longer excludes every directory named build, and enables additional lint rules. The PR also includes a broad set of mechanical fixes across Python scripts to comply with the tightened lint configuration.
Changes:
- Update
pyproject.tomlruff excludes to target specific artifact directories instead of the genericbuild, and enablePLE+RSErules. - Apply lint-driven refactors across Python code (exception raising simplifications, comprehension/list usage simplifications, logging via module-level loggers).
- Add/extend build-script support code and tests (e.g., glob matcher unit test, new build target definitions and Genio builder).
Reviewed changes
Copilot reviewed 28 out of 31 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/controller/python/tests/scripts/cluster_objects.py | Simplify raise ValueError() to satisfy exception-style lint rules. |
| src/controller/python/matter/crypto/p256keypair.py | Simplify NotImplementedError raising for abstract methods. |
| src/controller/python/matter/commissioning/init.py | Simplify NotImplementedError raising for abstract provider method. |
| src/controller/python/matter/clusters/ClusterObjects.py | Simplify NotImplementedError raising in cluster object base APIs. |
| scripts/py_matter_idl/matter/idl/lint/type_definitions.py | Simplify raising of MissingIdlError when IDL is absent. |
| scripts/py_matter_idl/matter/idl/generators/storage.py | Simplify NotImplementedError raising for storage interface. |
| scripts/flashing/psoc6_firmware_utils.py | Simplify NotImplementedError raising in interface methods. |
| scripts/flashing/firmware_utils.py | Simplify NotImplementedError raising in interface method. |
| scripts/build/test_glob_matcher.py | Add unit tests for GlobMatcher. |
| scripts/build/test.py | Replace list comprehensions with list(...) for lint compliance and minor clarity. |
| scripts/build/runner/shell.py | Use module-level logger; fix boolean logic (dedup and ...). |
| scripts/build/clang_coverage_wrapper.py | Use module-level logger; simplify iterable construction for Click choice. |
| scripts/build/builders/tizen.py | Use module-level logger; simplify exception suppression. |
| scripts/build/builders/telink.py | Use module-level logger for build output messages. |
| scripts/build/builders/nxp.py | Use module-level logger for warnings. |
| scripts/build/builders/nuttx.py | Use module-level logger for build output messages. |
| scripts/build/builders/nrf.py | Use module-level logger for exceptions/errors/info. |
| scripts/build/builders/genio.py | Add new Genio builder implementation. |
| scripts/build/builders/esp32.py | Use module-level logger for build output messages. |
| scripts/build/builders/efr32.py | Use module-level logger for build output messages. |
| scripts/build/builders/builder.py | Use module-level logger; simplify NotImplementedError raising; logging call sites updated. |
| scripts/build/builders/bouffalolab.py | Use module-level logger; simplify boolean expressions; update logging call sites. |
| scripts/build/builders/android.py | Use module-level logger; minor loop simplification and logging call updates. |
| scripts/build/build_examples.py | Minor lint-driven cleanups (unused helper removal, set comprehension). |
| scripts/build/build_darwin_framework.py | Simplify exception suppression via contextlib.suppress. |
| scripts/build/build/targets.py | Add/define build target matrix used by build scripts. |
| scripts/build/build/target.py | Use module-level logger; simplify loops/expressions; small helper cleanup. |
| scripts/build/build/init.py | Use module-level logger; adjust imports and logging call sites. |
| pyproject.toml | Refine ruff exclude patterns; enable PLE and RSE. |
| credentials/generate_revocation_set.py | Fix logging argument formatting to avoid logging runtime formatting errors. |
| build/config/linux/pkg-config.py | Simplify loop to any(...) for lint compliance. |
enkiusz
approved these changes
Mar 13, 2026
|
PR #43572: Size comparison from fa956ce to a83dd02 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43572 +/- ##
=======================================
Coverage 54.07% 54.07%
=======================================
Files 1548 1548
Lines 106709 106709
Branches 13308 13308
=======================================
Hits 57704 57704
Misses 49005 49005 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
buildname is a very generic name which appears in our Python modules and also is a directory for built artifacts. Instead of excluding a generic name (which will disable ruff check in some part of our code base) exclude specific paths.Also, added two new rules to ruff:
PLE(this rule has found 2 extra errors) andRSEfor simplifying exceptions.Testing
CI will verify.