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

contrib/aws/aws-sdk-go: expose handler names as const #1524

Merged
merged 5 commits into from
Oct 21, 2022
Merged

contrib/aws/aws-sdk-go: expose handler names as const #1524

merged 5 commits into from
Oct 21, 2022

Conversation

mstumpfx
Copy link
Contributor

I am directly modifying handlers in the request stack, and referencing them ByName will be safer if these are exposed.

Reference: #1168 (comment)

@mstumpfx mstumpfx requested a review from a team October 19, 2022 15:50
@knusbaum knusbaum added this to the v1.44.0 milestone Oct 19, 2022
@knusbaum
Copy link
Contributor

I don't see any reason not to expose these.

Copy link
Contributor

@knusbaum knusbaum left a comment

Choose a reason for hiding this comment

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

Ahh, forgot this nit.

contrib/aws/aws-sdk-go/aws/aws.go Outdated Show resolved Hide resolved
Copy link
Contributor

@knusbaum knusbaum left a comment

Choose a reason for hiding this comment

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

LGTM.

@DataDog DataDog deleted a comment from codecov bot Oct 19, 2022
@knusbaum
Copy link
Contributor

knusbaum commented Oct 19, 2022

Tests actually passed, they just failed to upload some results. This is fixed in another PR. Since the tests are mandatory, we have to wait for it.

@mstumpfx
Copy link
Contributor Author

Thanks for the quick review!

@ajgajg1134 ajgajg1134 merged commit 6a342a1 into DataDog:main Oct 21, 2022
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.

3 participants