-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(scheduler-targets-alpha): rename KinesisDataFirehosePutRecord
to FirehosePutRecord
#33758
Conversation
…isfirehose`
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33758 +/- ##
=======================================
Coverage 82.38% 82.38%
=======================================
Files 120 120
Lines 6937 6937
Branches 1170 1170
=======================================
Hits 5715 5715
Misses 1119 1119
Partials 103 103
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
In other modules, integration classes/methods are generally named
|
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.
Can we rename to just FirehosePutRecord
? To be consistent with the scheduler docs: https://docs.aws.amazon.com/scheduler/latest/UserGuide/managing-targets-templated.html. Would be great if we could merge this in soon as well, since we are planning to graduate this module soon and this is a breaking change. Thanks!
KinesisDataFirehosePutRecord
to DataFirehosePutRecord
KinesisDataFirehosePutRecord
to FirehosePutRecord
@gracelu0 Thank you for checking out. |
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.
is there a reason this file is included here?
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.
from a0cfdeb
a failure of git merge?
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.
restored the file from main.
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.
ah, my bad in resolving merge conflict.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
thank you!
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Related to #33757 and #33798
Reason for this change
Kinesis Data Firehose is now Amazon Data Firehose.
Therefore the class
KinesisDataFirehosePutRecord
should not be called Kinesis.Description of changes
KinesisDataFirehosePutRecord
toFirehosePutRecord
Describe any new or updated permissions being added
Nothing changed.
Description of how you validated changes
Updated tests and integ tests.
Checklist
BREAKING CHANGE: The class
KinesisDataFirehosePutRecord
has been renamed toFirehosePutRecord
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license