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

Add protocol tests for sparse list primitives #2333

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

adamthom-amzn
Copy link
Contributor

Background

  • What do these changes do?
    Adds a list of sparse shorts to existing sparse list tests

  • Why are they important?
    In some languages, like Java, Strings are naturally nullable and thus handling sparse collections of String values is straightforward, but sparse collection of values with primitives (such as shorts) may introduce complexities.

Testing

  • How did you test these changes?
    Ran these tests to verify that serde in a language implementation was broken, and then fixed the implementation and verified that tests pass.

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

In some languages, like Java, Strings are naturally nullable and thus
handling sparse collections of String values is straightforward, but
sparse collection of values with primitives (such as shorts) may
introduce complexities.
@adamthom-amzn adamthom-amzn requested a review from a team as a code owner June 19, 2024 17:35
Copy link
Contributor

@JordonPhillips JordonPhillips left a comment

Choose a reason for hiding this comment

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

Doing the same for maps would be great, though I'm not going to hold back this PR on that

@adamthom-amzn
Copy link
Contributor Author

Doing the same for maps would be great, though I'm not going to hold back this PR on that

We already have them; that's why I knew to look again at list protocol tests, because the map ones failed and the list ones did not.

@JordonPhillips JordonPhillips merged commit 1249c68 into smithy-lang:main Jun 20, 2024
13 checks passed
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