Skip to content

Conversation

@Graveflo
Copy link
Contributor

@Graveflo Graveflo commented Nov 5, 2025

Seems better to change it to CatchableError instead?

@Graveflo Graveflo marked this pull request as draft November 5, 2025 17:10
@Graveflo
Copy link
Contributor Author

Graveflo commented Nov 5, 2025

At this point I'm thinking that warnBareExcept (semstmt.nim : 353) should have a different name. (semstmt.nim : 397) should probably be silenced unless --legacy:noPanicOnExcept is enabled after RFC 557 was implemented.

@Graveflo
Copy link
Contributor Author

Graveflo commented Nov 5, 2025

This seems like the sanest path forward to me. I think asyncmacros needs to catch Exception and so does unittest, so does plenty of other things. It's just a warning right? So then an std module like unittest shouldn't just force it on. That's annoying. People who use it or depend on it for legitimate reasons need to be able to turn that off. As for the name, it seems to have precedence, so just leave it be.

Turned off the "The bare except clause is deprecated" warning when noPanicOnExcept is not enabled as I believe that is wrong now. Unfortunate how that message actually fits the name though.

CI might still fail though ofc.

@Graveflo Graveflo marked this pull request as ready for review November 8, 2025 08:56
@Graveflo Graveflo changed the title silence mass dump of BareExcept when using asyncdispatch silence mass dump of BareExcept when using unittest Nov 8, 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