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

feat: add per-rule granular error suppression #1826

Merged
merged 8 commits into from
Nov 7, 2024

Conversation

cgatt
Copy link
Contributor

@cgatt cgatt commented Oct 29, 2024

Fixes #1818

This does slightly change some behaviours, specifically by suppressing errors for suppressed rules. I'm torn as to whether this is a sufficiently large behavioural change to cause an issue, though as a rule I feel it is a better behaviour and cant think of a scenario where someone would have been relying on a suppressed rule throwing an error.

@dontirun
Copy link
Collaborator

though as a rule I feel it is a better behaviour and cant think of a scenario where someone would have been relying on a suppressed rule throwing an error.

Agreed! Thanks for the contribution. Can you add an example for suppressing validation failures to the README?

@cgatt
Copy link
Contributor Author

cgatt commented Nov 4, 2024

though as a rule I feel it is a better behaviour and cant think of a scenario where someone would have been relying on a suppressed rule throwing an error.

Agreed! Thanks for the contribution. Can you add an example for suppressing validation failures to the README?

Updated the readme, and while I was at it realised that I should probably still respect Suppression Ignore Conditions when suppressing validation failures, so slightly refactored to ensure that occurs and added some tests to that effect.

Comment on lines +7 to +12
interface ExpectMessageConditions {
readonly containing?: string[];
readonly notContaining?: string[];
readonly length?: number;
}
export function expectMessages(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

.projen/tasks.json Outdated Show resolved Hide resolved
@mergify mergify bot merged commit 86917b3 into cdklabs:main Nov 7, 2024
12 checks passed
@cgatt cgatt deleted the cgatt/granular-error-suppression branch November 10, 2024 23:44
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.

feat: CdkNagValidationFailure should use failing rule's ruleId as findingId
2 participants