Skip to content

fix(lambda-events): derive Default on various Events #1022

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

Merged
merged 3 commits into from
Jul 22, 2025
Merged

Conversation

pmd3d
Copy link
Contributor

@pmd3d pmd3d commented Jul 19, 2025

Issue #1009

Add derive default to various event types. CloudFormationCustomResourceRequest has each enum with a type parameter so manually wrote a default trait implementation that did not need a type parameter.

🔏 By submitting this pull request

  • I confirm that I've ran cargo +nightly fmt.
  • [skipped this because it changed code outside scope of the issue] I confirm that I've ran cargo clippy --fix.
  • I confirm that I've made a best effort attempt to update all relevant documentation.
  • I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@jlizen jlizen left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This seems reasonable to me.

These default impls are bit opinionated with the defaults for the enum variants, but, in practice they are only really useful for tests or allowing us to add (#[non-exhaustive] annotations](#1016) to use with future (Optional) fields. It's not like all of these String fields defaulting to "" would be functional anyway. I don't see a great alternative to this approach that would still allow us to get out of the pit of semver hazards as fields are gradually added.

I would be more concerned around default implementations on response structs since consumers would actually be constructing those directly in their binaries. And, that's not part of this PR.

@jlizen jlizen merged commit baf3653 into awslabs:main Jul 22, 2025
8 checks passed
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