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

Re-point those using ByteStream and SdkBody to smithy-types #3076

Conversation

ysaito1001
Copy link
Contributor

@ysaito1001 ysaito1001 commented Oct 17, 2023

Motivation and Context

A follow-up on #3026

Description

#3026 moved

  • aws_smithy_http::body::{BoxBody, Error, SdkBody} to aws_smithy_types::body::{BoxBody, Error, SdkBody}
  • aws_smithy_http::byte_stream::{AggregatedBytes, ByteStream, error::Error} to aws_smithy_types::byte_stream::{AggregatedBytes, ByteStream, error::Error}

and also left "breadcrumbs", so that customers could still consume updated types from aws_smithy_http after the move.

This PR turns breadcrumbs into deprecation messages (via #[deprecated(...)]) and updates existing places that used to use moved types from aws_smithy_http to directly depend on aws_smithy_types.

Testing

Relied on tests in CI.

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions
Copy link

@github-actions
Copy link

@ysaito1001 ysaito1001 marked this pull request as ready for review October 18, 2023 15:25
@ysaito1001 ysaito1001 requested review from a team as code owners October 18, 2023 15:25
@ysaito1001 ysaito1001 requested a review from unexge October 18, 2023 15:25
"SmithyHttpServer" to ServerCargoDependency.smithyHttpServer(runtimeConfig).toType(),
"SmithyTypes" to RuntimeType.smithyTypes(runtimeConfig),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I am not 100% sure on how .resolve works. But I am assuming, that is the way a type should be referred to in the code generator instead of hard coding it like #{SmithyTypes}::byte_stream::ByteStream, which is how we've done in streamingBodyTraitBounds.

So instead of SmithyTypes, if we keep "ByteStream" to RuntimeType.smithyTypes(rc).resolve("byte_stream::ByteStream") in the codegenScope do you think that would resolve into the same generated code?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah that results in the same generated code

@ysaito1001 ysaito1001 enabled auto-merge October 19, 2023 19:48
@github-actions
Copy link

@ysaito1001 ysaito1001 added this pull request to the merge queue Oct 19, 2023
Merged via the queue into main with commit ed8763e Oct 19, 2023
40 of 41 checks passed
@ysaito1001 ysaito1001 deleted the ysaito/follow-up-on-moving-sdkbody-and-bytestream-to-smithy-types branch October 19, 2023 20:39
github-merge-queue bot pushed a commit that referenced this pull request Oct 25, 2023
…#3091)

## Motivation and Context
#3076 missed type alias for
some types we moved to `aws-smithy-types`. This small PR will fix that.

## Testing
Relied on the existing tests in CI

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will require a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants