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

Update example code to use write_all_buf method #408

Merged
merged 1 commit into from May 24, 2021
Merged

Conversation

LMJW
Copy link
Contributor

@LMJW LMJW commented May 24, 2021

Small PR, related to #349
*This PR updated the example code to use the lastest supported method write_all_buffrom AsyncWriteExt trait (tokio-rs/tokio#3737) to make example less verbose *

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

Test: ./gradlew :aws:sdk:test passed locally.

BUILD SUCCESSFUL in 8m 41s

Please let me know if you have any feedbacks. Thanks!

Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

LGTM! Can you change the Tokio dependency in smithy-http to 1.6? That will ensure that this builds for everyone

@LMJW
Copy link
Contributor Author

LMJW commented May 24, 2021

Absolutely. Let me do that.

@LMJW
Copy link
Contributor Author

LMJW commented May 24, 2021

Test:

  • Rerun ./gradlew :aws:sdk:assemble
  • Run ./gradlew :aws:sdk:cargoCheck
  • Run ./gradlew :aws:sdk:test BUILD SUCCESSFUL in 7m 52s

All successful.

@LMJW LMJW requested a review from rcoh May 24, 2021 13:21
Copy link
Collaborator

@rcoh rcoh left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution!

@rcoh rcoh enabled auto-merge (squash) May 24, 2021 13:27
@rcoh rcoh disabled auto-merge May 24, 2021 13:30
@rcoh
Copy link
Collaborator

rcoh commented May 24, 2021

Looks like the build failed for a cargofmt check—can you run cargo fmt in the polly synthesize speech example? I will document that more clearly in the contribution docs

@LMJW
Copy link
Contributor Author

LMJW commented May 24, 2021

Okay. Let me update it. I thought the test would have caught the fmt failure :P. But I guess I should've run cargo fmt after I made the change. :)

@rcoh
Copy link
Collaborator

rcoh commented May 24, 2021

yeah I think that's an oversight that I'll fix. In the future, installing pre-commit will run all of our precommit hooks (including cargo fmt)

@LMJW
Copy link
Contributor Author

LMJW commented May 24, 2021

Just updated.
Rerun ./gradlew :aws:sdk:assemble
Run ./gradlew :aws:sdk:cargoCheck
Run ./gradlew :aws:sdk:test BUILD SUCCESSFUL in 7m 55s

@LMJW
Copy link
Contributor Author

LMJW commented May 24, 2021

Hi @rcoh Looks like the workflow needs to be approved to run

@rcoh rcoh enabled auto-merge (squash) May 24, 2021 14:28
@rcoh rcoh merged commit d8eaa32 into smithy-lang:main May 24, 2021
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.

None yet

2 participants