-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Add kotlin code snippets to spring-boot refdoc #18017
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
Conversation
FYI this PR is work in progress, more commits will come for other asciidoc files and I plan to review them. |
spring-boot-project/spring-boot-docs/src/main/asciidoc/using-spring-boot.adoc
Outdated
Show resolved
Hide resolved
spring-boot-project/spring-boot-docs/src/main/asciidoc/using-spring-boot.adoc
Outdated
Show resolved
Hide resolved
Thanks for the PR, @IEnoobong. Before too much more time is invested in this, I think we should pause and consider #6313. That issue is already quite a big job and inlining Kotlin snippets in the documentation will make it bigger. |
Hey @wilkinsona, would you be ok if we provide Kotlin code samples as runnable code directly? I think providing a consistent experience across Boot and Framework reference documentation would bring a lot of value to developers (see how it looks like on here where core, testing, WebFlux and WebMvc documentations are translated). |
That sounds good to me, assuming that we don't run into Eclipse-related problems with the Kotlin source. We've been fine with the Kotlin code that we've got thus far so I don't anticipate any problems. A nice side-effect would be that it might give us the necessary impetus to do the same for our Java examples. |
sync forks
...s/src/main/java/org/springframework/boot/docs/autoconfigure/KotlinManualAutoConfiguration.kt
Outdated
Show resolved
Hide resolved
...oot-docs/src/main/java/org/springframework/boot/docs/service/KotlinDatabaseAccountService.kt
Outdated
Show resolved
Hide resolved
@IEnoobong After a second thought, I think you should use a Kotlin specific package but same class names. So instead of:
Something like:
Could you please modify the PR accordingly? |
@sdeleuze Alright, I’m thinking to prefer service/kotlin to kotlin/service. What do you think? |
That's ok |
Yes, there's an example on immutable binding on line 898 |
private final Security security; | ||
|
||
public AcmeProperties(boolean enabled, InetAddress remoteAddress, Security security) { | ||
public AcmeProperties(boolean enabled, InetAddress remoteAddress, int port, Security security) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added port to enable instantiation of the server in https://github.com/spring-projects/spring-boot/pull/18017/files#diff-51b04ca54a1bd6f28cae0525b2b1a7fdR23
works with No there are not. |
It will be too short for Spring Boot 2.2, I propose we target Spring Boot 2.3 for this PR instead (if possible beginning of the dev cycle in order to avoid too much merge conflicts). Maybe that will be also the opportunity to move forward on #6313 for Java as well in Spring Boot 2.3 timeframe if Boot team think it could be in the roadmap for next year. |
A pre-requisite of this PR was that our code samples move first to code snippet so that we figure out what to do to provide a Kotlin counterpart. This PR has been sitting for a while and we'd like to to address the first part, see #6313. Thanks for the PR and sorry for the wasted time. |
Add kotlin code snippets to spring-boot refdoc
See gh-21778