-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update CBOR to always have an accept header #219
Conversation
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.
Worth adding a changelog entry for?
Yes. Still needs Smithy update. |
@@ -1,6 +1,8 @@ | |||
Unreleased Changes | |||
------------------ | |||
|
|||
* Issue - Support services migrating from AWS Query to AWS JSON or RPC v2. |
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.
This AWS specific bit is kind of weird to have in our generic changelog.
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.
Yeah. I can change this "from AWS Query to RPC v2" - the query trait (AWS) is used generically now so that bit must stay.
TreeSet<Shape> shapesToRender = CodegenUtils.getAlphabeticalOrderedShapesSet(); | ||
TreeSet<Shape> eventStreamEventsToRender = CodegenUtils.getAlphabeticalOrderedShapesSet(); | ||
|
||
Comparator<Shape> comparator = Comparator.comparing(o -> o.getId().getName() + " " + o.getId()); |
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.
Why this change? The codegenutils helper method for this seems cleaner than implementing a comparator for each one?
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.
It was the only method in the utility after unused methods cleaned up. It felt weird and was used in 3 classes. I think it's fine to duplicate it here as we've effectively duplicated the same logic for iterating shapes.
Needs to update Smithy and test against protocol tests.