- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.1k
Upgrade to Dokka 2.1.0 #10507
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
Upgrade to Dokka 2.1.0 #10507
Conversation
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.
Thank you for a swift turn around!
Not too much, just couple nit-picks and some concerns.
Can you share, please, with me a screenshot how the final doc looks?
After this we would be able to migrate to Gradle 9.1 finally 😄
        
          
                build.gradle
              
                Outdated
          
        
      | tasks.register('docsZip', Zip) { | ||
| dependsOn 'dokkaHtmlMultiModule' | ||
| dependsOn 'dokkaGeneratePublicationHtml' | ||
| dependsOn ':spring-integration-core:dokkaGenerate' | 
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.
Why is this?
Doesn't sound right place for such a dependency.
What happened to multi in Dokka?
Why it cannot collect all the sub-projects as before any more?
Not directly a question to you, but we need to figure out how to get rid of this explicit dependency.
Imaging now that would have Kotlin API in other modules.
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.
We only need to run the documentation in this module.
7d25d2f    to
    29b169d      
    Compare
  
    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.
This is what Spring Framework does:
https://github.com/spring-projects/spring-framework/blob/main/framework-api/framework-api.gradle
And this line is strange for me: https://github.com/spring-projects/spring-framework/blob/main/buildSrc/src/main/java/org/springframework/build/KotlinConventions.java#L40
        
          
                build.gradle
              
                Outdated
          
        
      |  | ||
| dokka { | ||
| dokkaPublications.html { | ||
| outputDirectory.set(file(rootProject.layout.buildDirectory.file('kdoc'))) | 
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.
Why do we need call file() twice?
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.
Why do we still call file(file())?
        
          
                build.gradle
              
                Outdated
          
        
      | apply plugin: 'kotlin' | ||
| apply plugin: 'kotlin-spring' | ||
| apply plugin: 'io.spring.nullability' | ||
| apply plugin: 'org.jetbrains.dokka' | 
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.
This is not correct: it is waste of resource to apply the plugin in the sub-project where is nothing with Kotlin.
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.
Looks like there is no way to make it transparent: https://kotlinlang.org/docs/dokka-migration.html#update-documentation-aggregation-in-multi-module-projects.
We have to be explicit with that our module:
dependencies {
    dokka(project(':spring-integration-core'))
}
in the root project 😢
29b169d    to
    e15a370      
    Compare
  
    | I smoothed it out a bit in this last commit. But yeah, they dropped that support. | 
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.
Strange. I thought I sent my review.
Sorry about that.
        
          
                build.gradle
              
                Outdated
          
        
      |  | ||
| dokka { | ||
| dokkaPublications.html { | ||
| outputDirectory.set(file(rootProject.layout.buildDirectory.file('kdoc'))) | 
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.
Why do we still call file(file())?
        
          
                build.gradle
              
                Outdated
          
        
      | javaProjects = subprojects - project(':spring-integration-bom') | ||
|  | ||
| // Modules that have Kotlin sources to document using dokka | ||
| dokkaProjects = [ | 
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.
Can we make it more smarted without such a variable?
        
          
                build.gradle
              
                Outdated
          
        
      | } | ||
|  | ||
| // Process only the modules that require kotlin docs | ||
| configure(subprojects.findAll { dokkaProjects.contains(it.name) }) { subproject -> | 
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.
We have already a configure above.
Such a Dokka configuration could be done there.
Can we go similar to SF way? Kinda check if there is sourceSets.main.kotlin, then apply Dokka config in that subproject?
        
          
                build.gradle
              
                Outdated
          
        
      |  | ||
| tasks.register('docsZip', Zip) { | ||
| dependsOn 'dokkaHtmlMultiModule' | ||
| dependsOn dokkaProjects.collect { ":${it}:dokkaGenerate" } | 
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.
Probably with dokka() as dependency like in SF, we still would need to have dokka in the root project and here that would be just ` dependsOn 'dokkaGenerate'
- Update `org.jetbrains.dokka` plugin from 2.0.0 to 2.1.0 - Remove Jackson 2.15.3 version resolution strategy workaround that was needed to avoid conflicts in Dokka 2.0.0 - Migrate from deprecated V1 tasks (`dokkaHtmlPartial`, `dokkaHtmlMultiModule`) to V2 API (`dokkaGeneratePublicationHtml`, `dokkaGenerate`) - Replace `dokkaSourceSets.main` configuration block with `dokkaSourceSets.configureEach` - Update `docsZip` task to depend on new `dokkaGeneratePublicationHtml` task and add explicit dependency on `:spring-integration-core:dokkaGenerate`
- Restore external documentation links in spring-integration-core's `dokkaSourceSets` using the new `externalDocumentationLinks. register()` API instead of deprecated `externalDocumentationLink` - Fix indentation of spring-integration-core's `dokka` configuration block to be properly scoped within the project - Remove redundant `noJdkLink.set(false)` setting (false is default) - Remove unnecessary root-level `dokka` configuration that was setting module name and output directory without effect - Remove unused `dokkaGeneratePublicationHtml` task dependency from `docsZip` as `dokkaGenerate` already handles the generation - Simplify dependency chain by relying solely on `:spring-integration-core:dokkaGenerate` - Use root project var to determine root - Move the out of core module This makes the dokka build available to all modules if necessary - Only run the module in the core module
- Refactor Dokka configuration to apply only to modules with Kotlin sources, reducing build overhead and improving performance for modules without Kotlin code. - Remove deprecated commands
Replace static `dokkaProjects` list with dynamic Kotlin source
detection, eliminating manual maintenance and improving automation.
- Remove `dokkaProjects` list from ext block that required manual
  updates when adding Kotlin modules
- Move Dokka plugin and configuration back into main `javaProjects`
  configure block for simpler structure
- Update `docsZip` task to dynamically find projects with Kotlin
  sources using `javaProjects.findAll {it.file('src/main/kotlin').
  exists()}` instead of referencing static list
- Change output directory API from `buildDirectory.file('kdoc')` to
  `buildDirectory.dir('kdoc')` for semantically correct directory
  reference
    e15a370    to
    74e2a59      
    Compare
  
    

org.jetbrains.dokkaplugin from 2.0.0 to 2.1.0dokkaHtmlPartial,dokkaHtmlMultiModule) to V2 API (dokkaGeneratePublicationHtml,dokkaGenerate)dokkaSourceSets.mainconfiguration block withdokkaSourceSets.configureEachdocsZiptask to depend on newdokkaGeneratePublicationHtmltask and add explicit dependency on:spring-integration-core:dokkaGenerate