Skip to content

fix: replace bare except with except Exception in analysis.py#1502

Merged
aaronpowell merged 1 commit intogithub:stagedfrom
srpatcha:fix/bare-except-analysis-v2
May 5, 2026
Merged

fix: replace bare except with except Exception in analysis.py#1502
aaronpowell merged 1 commit intogithub:stagedfrom
srpatcha:fix/bare-except-analysis-v2

Conversation

@srpatcha
Copy link
Copy Markdown
Contributor

Summary

Replaces bare \�xcept:\ with \�xcept Exception:\ in \skills/publish-to-pages/scripts/convert-pdf.py\ analysis function to avoid catching \SystemExit\ and \KeyboardInterrupt.

Re-targeted to \staged\ branch as requested in #1500.

Signed-off-by: Srikanth Patchava [email protected]

Bare except clauses catch SystemExit and KeyboardInterrupt which
prevents clean process termination. Use except Exception per PEP 8.
@srpatcha srpatcha requested a review from aaronpowell as a code owner April 25, 2026 08:03
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 25, 2026

🔍 Skill Validator Results

✅ All checks passed

Scope Checked
Skills 1
Agents 1
Total 2
Severity Count
--- ---:
❌ Errors 0
⚠️ Warnings 0
ℹ️ Advisories 0

Summary

Level Finding
ℹ️ Found 1 skill(s)
ℹ️ [datanalysis-credit-risk] 📊 datanalysis-credit-risk: 1,365 BPE tokens [chars/4: 1,546] (detailed ✓), 16 sections, 1 code blocks
ℹ️ ✅ All checks passed (1 skill(s))
Full validator output ```text Found 1 skill(s) [datanalysis-credit-risk] 📊 datanalysis-credit-risk: 1,365 BPE tokens [chars/4: 1,546] (detailed ✓), 16 sections, 1 code blocks ✅ All checks passed (1 skill(s)) ```

Copy link
Copy Markdown
Contributor

@aaronpowell aaronpowell left a comment

Choose a reason for hiding this comment

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

The test-plugins scripts are unrelated to the PR description, and we already have scripts in the repo related to validating plugins.

@srpatcha
Copy link
Copy Markdown
Contributor Author

Hi @aaronpowell, thank you for the feedback! You're right — the test-plugins scripts are unrelated to the bare except fix. I'll remove those commits and keep this PR focused on the except fix only. I can submit the validation framework as a separate PR if there's interest.

@aaronpowell
Copy link
Copy Markdown
Contributor

We already have validation in eng/validate-plugins.mjs script, plus we have an external tool that runs validations as part of CI.

@srpatcha srpatcha force-pushed the fix/bare-except-analysis-v2 branch from 95e5473 to 722163b Compare May 5, 2026 01:38
@srpatcha
Copy link
Copy Markdown
Contributor Author

srpatcha commented May 5, 2026

Hi @aaronpowell, I've restructured the PR to keep only the original bare-except fix in \scripts/eval/analysis.py. The validation framework and unrelated commits have been removed. The PR now contains exactly one focused change. Ready for re-review!

@github-actions github-actions Bot added the skills PR touches skills label May 5, 2026
@aaronpowell aaronpowell merged commit 769741b into github:staged May 5, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skills PR touches skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants