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

Use field.getUnlessDefault instead of field.get in UrlFormDataEncoder #1323

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

bpholt
Copy link
Contributor

@bpholt bpholt commented Dec 8, 2023

Fixes #1321.

PR Checklist (not all items are relevant to all PRs)

  • Added unit-tests (for runtime code)
  • Added bootstrapped code + smoke tests (when the rendering logic is modified)
  • Added build-plugins integration tests (when reflection loading is required at codegen-time)
  • Added alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)
  • Updated dynamic module to match generated-code behaviour
  • Added documentation
  • Updated changelog

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@bpholt
Copy link
Contributor Author

bpholt commented Dec 8, 2023

@Baccata could you help me figure out where to add unit tests for this change? It's unclear to me where they belong—maybe there aren't any unit tests for UrlFormDataEncoder yet?

@Baccata
Copy link
Contributor

Baccata commented Dec 8, 2023

https://github.com/disneystreaming/smithy4s/blob/series/0.18/modules/bootstrapped/test/src/smithy4s/http/internals/UrlFormDataEncoderDecoderSchemaVisitorSpec.scala

The unit tests of the core module live in the bootstrapped module, which makes it easier to write assertions against the generated code

@bpholt bpholt marked this pull request as ready for review January 2, 2024 21:12
Copy link
Member

@kubukoz kubukoz left a comment

Choose a reason for hiding this comment

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

Please add a CHANGELOG entry for 0.18.5. Other than that, looks good to me

@Baccata Baccata merged commit cebdd43 into disneystreaming:series/0.18 Jan 3, 2024
10 of 11 checks passed
@bpholt bpholt deleted the issue-1321 branch March 15, 2024 20:47
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.

Serializing default values causes failure in EC2 DescribeRequest
4 participants