Skip to content

Conversation

@gasolima
Copy link
Contributor

Issue # (if applicable)

N/A

Reason for this change

onEvent() has a lot of options other than description e.g. ruleName detail etc..
The old code didn't pass those values to rule, so it will never be propagated properly, so the consumer of this method will think that we're respecting their options, but in real we're discarding it.

Description of changes

Creating the rule first with all the options so all the options from the consumer get propagated properly to the rule, then add the specific things that you want.

Describe any new or updated permissions being added

N/A

Description of how you validated changes

Add unit tests

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team October 23, 2025 13:10
@github-actions github-actions bot added the p2 label Oct 23, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 23, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

@gasolima gasolima force-pushed the fix-onEvent branch 2 times, most recently from c8d5581 to 8b64ccc Compare October 24, 2025 12:58
@gasolima gasolima marked this pull request as ready for review October 24, 2025 13:36
@gasolima gasolima changed the title fix: onEvent function passes all the options to rule resource fix: onEvent function to pass all the options to rule resource Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants