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

Limit streaming traits to top level members #221

Merged

Conversation

JordonPhillips
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@JordonPhillips JordonPhillips requested a review from kstich December 2, 2019 18:42
@mtdowling
Copy link
Member

Why do we need this? Ideally we could apply streaming traits to only shapes or only members.

The motivation for applying only to shapes is that the streaming trait should change the code generated type of the shape. We usually try to not have members alter the type that is generated in targeted programming languages.

However, an argument for the streaming trait to only be applied to members could be made based on the fact that only top-level input, output, and error shapes can every be streaming. These are actually the same locations as event streams. The rationale for making even streams apply to the member of a structure was that the structure that targets the event stream is essentially claimed as a special shape that can't be used like other shapes throughout the model.... Maybe we should apply this same treatment to the streaming trait.

@JordonPhillips JordonPhillips changed the title Allow streaming trait to apply to members Limit streaming traits to top level members Dec 5, 2019
A streaming blob on a nested structure doesn't make much sense, so
this updates the streaming trait to only apply to members of
operation input/output and errors.

Fundamentally this doesn't change the way that serde code would be
generated, since at the end of the day simple types like blobs will
never get generated types.
@JordonPhillips JordonPhillips merged commit 5dcbbc5 into smithy-lang:master Dec 6, 2019
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.

2 participants