-
Notifications
You must be signed in to change notification settings - Fork 2.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
chore(auth): general clean up for credentialsProvider and also rename Credentials from sdk #12021
chore(auth): general clean up for credentialsProvider and also rename Credentials from sdk #12021
Conversation
export type { | ||
AwsCredentialIdentity as Credentials, | ||
MetadataBearer, | ||
} from '@aws-sdk/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.
are we doing the thing that Allan did on v5 to generate those and have @aws-sdk/types
as dev dependency
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.
+1
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.
Okay, we can create a follow up PR for this. Thanks
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.
Looks good!
qq: what about generating the types instead of making it a dependency
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 think we should add a todo to auto generate the types from the SDK. Other than that, LGTM.
export type { | ||
AwsCredentialIdentity as Credentials, | ||
MetadataBearer, | ||
} from '@aws-sdk/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.
+1
Description of changes
Issue #, if available
Description of how you validated changes
Works as expected for the following scenarios on a React app,
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.