Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block PRs that increase the amount of panics in non-test code #7221

Merged
merged 5 commits into from
Nov 16, 2024

Conversation

tannerntannern
Copy link
Contributor

Implements @Anton-4's idea on #2046 for automatically checking when the number of panics/expects/etc., increases.

Clippy doesn't scan test code by default, so this won't get in the way of using unwrap in tests, to @rtfeldman's point. I also manually verified this by adding/removing unwraps within and outside of test modules, and verifying that the count went up or down as expected.

I wasn't sure if I should make this new workflow be triggered by ci_manager.yml or if it's fine to be triggered on every PR pipeline. At least one other workflow is like this (spellcheck.yml) so I went with the latter, but I'm happy to change it however you'd like.

@tannerntannern tannerntannern force-pushed the prohibit-increasing-panics branch 2 times, most recently from c7cf0b8 to 85454ca Compare November 15, 2024 16:26
@tannerntannern tannerntannern force-pushed the prohibit-increasing-panics branch from 85454ca to 331ff4c Compare November 15, 2024 18:13
@Anton-4 Anton-4 self-assigned this Nov 16, 2024
Copy link
Collaborator

@Anton-4 Anton-4 left a comment

Choose a reason for hiding this comment

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

Thanks @tannerntannern!
I moved the workflow under CI-manager and improved some of the names in the script. I also removed the unreachable check because it seems like there are valid use-cases of that one, time will tell I suppose :)

@Anton-4 Anton-4 merged commit 434836b into roc-lang:main Nov 16, 2024
19 checks passed
@tannerntannern tannerntannern deleted the prohibit-increasing-panics branch November 16, 2024 21:09
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