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

Upgrade to Smithy 1.26.x #1920

Closed
wants to merge 4 commits into from
Closed

Upgrade to Smithy 1.26.x #1920

wants to merge 4 commits into from

Conversation

syall
Copy link
Contributor

@syall syall commented Nov 11, 2022

BEFORE MERGING: Make sure client serializers have no merge conflicts.

Description of changes:

The following changes are included in this PR:

  1. Upgrade to smithy 1.26.x using a version range.

  2. Update serializing empty lists for query protocols (see references) to pass protocol tests.

    Previous Behavior:

    { "ListArg": [] } serializes to ``.

    Current Behavior:

    { "ListArg": [] } serializes to ?ListArg=.

  3. Protocol tests are regenerated

  4. Client serializers that use query protocols are regenerated

    • This is NOT a breaking change, but a bug fix that doesn't impact user code.

References:


Testing:

  1. make
  2. make unit-modules-internal_protocoltest

For changes to files under the /codegen/aws-models folder, and manual edits to autogenerated code (e.g. /service/s3/api.go) please create an Issue instead of a PR for those type of changes.

N/A


If there is an existing bug or feature this PR is answers please reference it here.

N/A


To help speed up the process and reduce the time to merge please ensure that Allow edits and access to secrets by maintainers is checked before submitting your PR. This will allow the project maintainers to make minor adjustments or improvements to the submitted PR, allow us to reduce the roundtrip time for merging your request.

N/A

@syall syall force-pushed the upgrade-smithy-1.26.x branch 2 times, most recently from ce0c9b9 to b4a289c Compare November 11, 2022 21:53
Steven Yuan added 2 commits November 11, 2022 13:54
Update serializing empty lists for query protocols (see references)
to pass protocol tests.

Previous Behavior:

`{ "ListArg": [] }` serializes to ``.

Current Behavior:

`{ "ListArg": [] }` serializes to `?ListArg=`.

References:

- smithy-lang/smithy#1444
- [AWS query
  protocol](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-query-protocol.html)
- [AWS EC2 query
  protocol](https://awslabs.github.io/smithy/2.0/aws/protocols/aws-ec2-query-protocol.html)
@syall syall force-pushed the upgrade-smithy-1.26.x branch from b4a289c to 96308bf Compare November 11, 2022 21:55
@syall syall force-pushed the upgrade-smithy-1.26.x branch from ebe428a to fe622a4 Compare November 11, 2022 22:42
@syall syall marked this pull request as ready for review November 11, 2022 22:57
@syall syall requested a review from a team as a code owner November 11, 2022 22:57
Comment on lines 2452 to 2456
if len(v) == 0 {
value.Array("member").Empty()
return nil
}
array := value.Array("member")
Copy link
Contributor Author

@syall syall Nov 12, 2022

Choose a reason for hiding this comment

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

Or maybe this generated code makes more sense?

Suggested change
if len(v) == 0 {
value.Array("member").Empty()
return nil
}
array := value.Array("member")
array := value.Array("member")
if len(v) == 0 {
array.Empty()
return nil
}

@syall
Copy link
Contributor Author

syall commented Jan 28, 2023

Closed in favor of #1996.

@syall syall closed this Jan 28, 2023
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.

1 participant