Skip to content

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Apr 28, 2025

Simplifies a lot of exception handling code

Slight overlap with #2285 (merging either first will reduce changes in a file)

I left exception-raising hacks for #2462

@Avasam Avasam force-pushed the Remove-old-style-sys.exc_info-usage-in-favor-of-caught-exception branch from 442fa31 to da55f70 Compare April 28, 2025 19:25
@mhammond
Copy link
Owner

I'm having trouble getting excited by these kinds of PRs - I agree it's arguably "better", and indeed if it was code written today we'd insist on it. However, I've not seen evidence that this fixes anything in practice but instead introduces risk of regressions and otherwise has marginal benefits, if any. I'd be in favour of changing code that's otherwise already being touched to have these kinds of improvments, but I'm not really in favor of general "improvement" changes with no clear justification other than "it's how it would have been written if it was written today". I'm even fine with auto-fix changes from quality tools in our CI (eg, via Ruff) but less motivated by self-selected and hand-written fixes.

@Avasam
Copy link
Collaborator Author

Avasam commented Apr 28, 2025

Alright, I'll close this PR (it stays as a reference point anyway). Some of those may come back up whilst adding annotations for editors/intellisence/type-checkers.

As a note for adodbapi/apibase.py (which is the overlap with #2285) specifically. That one allows re-enabling a check that prevents potential regressions and promotes the "better" way of writing it in some situations.

@Avasam Avasam closed this Apr 28, 2025
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