Skip to content

Conversation

@santigimeno
Copy link
Member

@santigimeno santigimeno commented Feb 3, 2026

Fixes this linting error:

PERF203 `try`-`except` within a loop incurs performance overhead

Summary by CodeRabbit

  • Chores
    • Improved installation resilience so failures in individual directories are handled locally and do not stop overall processing.
    • Refactored error handling to ensure setup continues across remaining directories, reducing interruptions during installation.

@santigimeno santigimeno self-assigned this Feb 3, 2026
@santigimeno santigimeno force-pushed the santi/fix_linting_install_py branch from e14a69f to f5ef8c0 Compare February 3, 2026 16:32
@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

Walkthrough

Introduces a per-directory safe_action wrapper in tools/install.py and refactors the deps/ncm-ng directory-walk to call actions through this wrapper, relocating exception handling from a global loop-level try/except to localized, per-call suppression so one directory's failure won't abort the whole traversal.

Changes

Cohort / File(s) Summary
Error handling / directory traversal
tools/install.py
Adds a safe_action wrapper that wraps individual directory actions in try/except; refactors the deps/ncm-ng walk to invoke actions via safe_action, moving error suppression from around the loop to per-directory calls and preserving existing path traversal logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 I hop through folders, one by one,
A gentle shield around each run,
If one should trip, I simply sigh—
I skip, I mend, I carry by.
Hooray for loops that never cry! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: avoid try/except inside a loop' directly aligns with the main change, which refactors code to move exception handling outside a directory-walk loop to address a linting/performance concern (PERF203).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch santi/fix_linting_install_py

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tools/install.py`:
- Around line 168-177: The safe_action wrapper currently uses a bare except
which hides all errors; change it to catch only Exception (e.g., except
Exception as e) and, if the installer is not in silent mode (check the silent
variable or its equivalent), log the exception (including e and context like
paths/destination and function name safe_action) before continuing; ensure you
still call action(options, paths, destination) inside the try/except and
preserve the original behavior of continuing the loop in the os.walk block that
builds paths and calls safe_action.

Fixes this linting error:
```
PERF203 `try`-`except` within a loop incurs performance overhead
```
@santigimeno santigimeno force-pushed the santi/fix_linting_install_py branch from f5ef8c0 to c4b26df Compare February 5, 2026 12:04
Copy link

@riosje riosje left a comment

Choose a reason for hiding this comment

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

LGTM

santigimeno added a commit that referenced this pull request Feb 9, 2026
Fixes this linting error:
```
PERF203 `try`-`except` within a loop incurs performance overhead
```

PR-URL: #419
Reviewed-By: Jefferson <[email protected]>
@santigimeno
Copy link
Member Author

Landed in c4442b3

@santigimeno santigimeno closed this Feb 9, 2026
@santigimeno santigimeno deleted the santi/fix_linting_install_py branch February 9, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants