-
Notifications
You must be signed in to change notification settings - Fork 966
Minor refacotring to AwsV4HttpSigner impl #4556
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
Minor refacotring to AwsV4HttpSigner impl #4556
Conversation
And other minor refactoring.
6956c2f to
b44f83d
Compare
|
SonarCloud Quality Gate failed.
|
| // TODO(sra-identity-auth): Consider if we can rename this to convey something more. maybe, | ||
| // V4RequestSigningResult/V4RequestSigningResultData? Note there is V4aContext similarly. |
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.
Yes, V4Context is a terrible name. I like V4RequestSigningResult or V4RequestSigningOutput
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.
will take up the TODOs separately.
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 don't think it needs to be a builder anymore with the payload-signer now having a dedicated method to modify the request that is ran before request-signing.
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.
yup, I noticed later it can be changed, but didn't take that on immediately. It's SdkInternal, so non-blocking, so was planing to address both TODOs later.
…d5007a114 Pull request: release <- staging/884f85a2-db64-4085-ab69-4acd5007a114












Motivation and Context
As I was reading code in this class, as part of debugging something, felt moving some code around made it easier to follow code. And maybe some style guide would recommend public methods earlier.
Modifications
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License