Skip to content

Kotlin: Add option to add defaults to nullable fields#863

Merged
kobylynskyi merged 5 commits into
kobylynskyi:developfrom
zmack:add-nullable-default
Nov 11, 2021
Merged

Kotlin: Add option to add defaults to nullable fields#863
kobylynskyi merged 5 commits into
kobylynskyi:developfrom
zmack:add-nullable-default

Conversation

@zmack

@zmack zmack commented Nov 8, 2021

Copy link
Copy Markdown
Contributor

Description

This adds the option to have kotlin classes generated with constructors that set the default value of nullable types set to null.

This would end up generating classes to the tune of

data class Commit(
    override
    val abbreviatedOid: String,
    val additions: Int,
    val associatedPullRequests: PullRequestConnection? = null,
    val author: GitActor? = null,
...

rather than

data class Commit(
    override
    val abbreviatedOid: String,
    val additions: Int,
    val associatedPullRequests: PullRequestConnection?,
    val author: GitActor?,
...

This ends up being convenient in Kotlin as it makes using constructors more terse and to some degree obviates the need for generating builders.

Please let me know if I missed adding anything here.

Thank you.

Changes were made to:

  • Codegen library - Java
  • Codegen library - Kotlin
  • Codegen library - Scala
  • Maven plugin
  • Gradle plugin
  • SBT plugin

@jxnu-liguobin

jxnu-liguobin commented Nov 9, 2021

Copy link
Copy Markdown
Collaborator

Welcome, it looks good, Let's wait until the test passes. 👍🏻

@zmack

zmack commented Nov 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@jxnu-liguobin a bit confused as to why that build's failing, all of the constants seem to be in the right place, am I missing something ?

Edit: Oh, I think I've got it, there's a version dependency there. It's not pulling the library from the project, it's pulling from the published project, so I need to land the change to the library before I can change the plugin 😢

kobylynskyi
kobylynskyi requested changes Nov 10, 2021
View reviewed changes
Comment thread docs/codegen-options.md Outdated
Comment thread ...gin/src/main/java/io/github/kobylynskyi/graphql/codegen/gradle/GraphQLCodegenGradleTask.java
Comment thread src/main/java/com/kobylynskyi/graphql/codegen/kotlin/KotlinGraphQLCodegen.java Outdated
@kobylynskyi

kobylynskyi commented Nov 10, 2021

Copy link
Copy Markdown
Owner

@zmack please change the target branch to develop (according to CONTRIBUTING.md#development-process)
Thanks.

@zmack zmack changed the base branch from master to develop November 10, 2021 06:01
@zmack zmack force-pushed the add-nullable-default branch from a49128a to 8710bb6 Compare November 10, 2021 06:02
@zmack zmack requested a review from kobylynskyi November 10, 2021 06:12
@zmack

zmack commented Nov 10, 2021

Copy link
Copy Markdown
Contributor Author

@zmack please change the target branch to develop (according to CONTRIBUTING.md#development-process)
Thanks.

Oh welp, apologies, I totally missed that. Updated the target branch and rebased and that took care of all my testing woes 🥳
Thank you.

kobylynskyi
kobylynskyi approved these changes Nov 11, 2021
View reviewed changes

@kobylynskyi kobylynskyi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@zmack looks good now!

@zmack

zmack commented Nov 11, 2021

Copy link
Copy Markdown
Contributor Author

@kobylynskyi is the sonar report failure due to one of my changes or unrelated?

@kobylynskyi

kobylynskyi commented Nov 11, 2021

Copy link
Copy Markdown
Owner

Nope, that's because Sonar run is not supported in PRs. You are good.

@kobylynskyi kobylynskyi merged commit 96bdde7 into kobylynskyi:develop Nov 11, 2021
@kobylynskyi kobylynskyi added this to the 5.3.1 milestone Nov 11, 2021
@Djaler

Djaler commented Jan 22, 2024 •
edited
Loading

Copy link
Copy Markdown

Sorry for necroposting, but do you intentionally not add default null in parameterized input template because of something, or this is a bug?
@zmack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kobylynskyi kobylynskyi kobylynskyi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

5.4.0

Development

Successfully merging this pull request may close these issues.

4 participants

@zmack @jxnu-liguobin @kobylynskyi @Djaler