-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore: rename Lambda related interfaces #288
Conversation
6bb35a2
to
feb86a7
Compare
src/constants.ts
Outdated
@@ -27,7 +27,7 @@ export enum RuntimeType { | |||
UNSUPPORTED, | |||
} | |||
|
|||
export const DefaultDatadogProps = { | |||
export const DefaultDatadogLambdaProps = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda prefer DatadogLambdaDefaultProps
but only because it falls in line with the naming of the other props types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find another default prop type. Could you give an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I think I wasn't clear. My (nitpick) preference is that this prop be renamed DatadogLambdaDefaultProps
rather than DefaultDatadogLambdaProps
. The other prop types I was referring to were:
DatadogLambdaStrictProps
DatadogLambdaProps
Notice that these both start with Datadog
, where as the default one doesn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense! Will rename it.
/** | ||
* For backward compatibility. It's recommended to use DatadogLambda for | ||
* users who want to add Datadog monitoring for Lambda functions. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we include the word "Deprecated" in this statement somewhere? This will tell us and users what to expect when we do our next major version update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will add "To be deprecated.".
feb86a7
to
feb6e4f
Compare
/merge |
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
This is a breaking change |
What does this PR do?
DatadogProps
->DatadogLambdaProps
DefaultDatadogProps
->DatadogLambdaDefaultProps
DatadogStrictProps
->DatadogLambdaStrictProps
export type DatadogProps = DatadogLambdaProps;
so the renaming won't affect usersMotivation
To make it cleaner to add
DatadogStepFunction
class later. Details in [RFC] Changing API for Datadog CDK ConstructTesting Guidelines
npm test
Additional Notes
Types of Changes
Check all that apply