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 generateInputBuilders #5146

Merged
merged 2 commits into from
Aug 3, 2023
Merged

Add generateInputBuilders #5146

merged 2 commits into from
Aug 3, 2023

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Aug 3, 2023

Kotlin constructors with default params are nice but when compared to Builders, they lack the ability to represent the "absent" case. Builders allow 3 cases:

// present
Builder()
  .param(42)
  .build()

// null
Builder()
  .param(null)
  .build()

// absent
Builder()
  .build()

The same code in Kotlin requires wrapping the parameters in Optional which is more verbose when there are a lot of parameters (See #4747):

// present
SomeClass(param = Optional.Present(42))

// null
SomeClass(param = Optional.Present(null))

// absent (default)
SomeClass()

This PR brings back builders for those cases

@martinbonnin martinbonnin requested a review from BoD as a code owner August 3, 2023 09:32
@netlify
Copy link

netlify bot commented Aug 3, 2023

Deploy Preview for apollo-android-docs ready!

Name Link
🔨 Latest commit 508b784
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/64cbb8e535d6710008fce00b
😎 Deploy Preview https://deploy-preview-5146--apollo-android-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

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

This is great! 🎉

The doc in plugin-configuration.mdx should be updated with the new option I guess :)

*
* Default: false
*/
@ApolloExperimental
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's one case where we could add a @Since?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea but I would only add @since once the symbol is stable?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yes good point, makes sense!

@martinbonnin
Copy link
Contributor Author

The doc in plugin-configuration.mdx should be updated with the new option I guess :)

Right, we live in that world now. Having this duplication isn't great but you're right, I'll add it there

@martinbonnin martinbonnin requested a review from a team as a code owner August 3, 2023 12:01
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