misc: upgrade to Dokka 2.0.0 #1284
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,30 @@ | |||
| import kotlin.text.set | |||
There was a problem hiding this comment.
Question: I like the idea of refactoring this out of the main build file. How much more work would it be to move it all the way out into aws-kotlin-repo-tools so that we had less duplication?
There was a problem hiding this comment.
It's possible to consume a conventions plugin (one defined in aws-kotlin-repo-tools) from another conventions plugin (one defined in smithy-kotlin).
We'd still need to have this dokka-convention plugin defined in smithy-kotlin and keep the dependencies { } block which is repo-specific.
I don't think it's worth commonizing the 20 lines of config that we have here at the moment
There was a problem hiding this comment.
Why? What about this Dokka configuration is unique to smithy-kotlin?
There was a problem hiding this comment.
Most of this is not unique to smithy-kotlin. This is:
dependencies {
dokkaPlugin(project(":dokka-smithy"))
}
The rest of the file could be commonized but then we'd still need a convention plugin defined in each repository
There was a problem hiding this comment.
aws-sdk-kotlin has an additional custom asset navigation-loader.js which would also need to be manually defined / deviated from the proposed commonized plugin. I think it's fine to leave them separate for now
| // Output subprojects' docs to <docs-base>/project-name/* instead of <docs-base>/path/to/project-name/* | ||
| // This is especially important for inter-repo linking (e.g., via externalDocumentationLink) because the | ||
| // package-list doesn't contain enough project path information to indicate where modules' documentation are | ||
| // located. | ||
| fileLayout.set { parent, child -> | ||
| parent.outputDirectory.dir(child.moduleName) |
There was a problem hiding this comment.
Does dokka handle this now or is the layout going to change?
There was a problem hiding this comment.
This didn't seem to be necessary anymore in my testing. It was an old bit of code, might have been fixed a long time ago or just now in Dokka 2.0.0
| dependencies { | ||
| dokka(project(":runtime")) | ||
| } |
There was a problem hiding this comment.
Why do we need this now?
There was a problem hiding this comment.
Do you mean the dokka(project(":runtime"))? This is how you aggregate multi-module documentation in Dokka 2: https://kotlinlang.org/docs/dokka-migration.html#update-documentation-aggregation-in-multi-module-projects
| * Ensure `navButton` elements are interactable and have proper accessibility properties | ||
| * Fixes accessibilty violation: "Ensure all interactive functionality is operable with the keyboard" | ||
| */ | ||
| function ensureNavButtonInteractable() { |
There was a problem hiding this comment.
Is this built-in in dokka 2?
There was a problem hiding this comment.
Yes it was fixed by them, you can pull down the docs and verify it yourself if interested
This comment has been minimized.
This comment has been minimized.
|
Just curious, I see there is a search button in header, is this a new feature by dokka2? Seems like it's not working at this moment. |
|
@xinsong-cui Good catch, search should be disabled, that button shouldn't be there. I'll take a look at why it's not getting disabled. |
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,30 @@ | |||
| import kotlin.text.set | |||
There was a problem hiding this comment.
Why? What about this Dokka configuration is unique to smithy-kotlin?
| "**/*.{kt,kts}", | ||
| "!**/generated-src/**", | ||
| "!**/smithyprojections/**", | ||
| "!**/build/**", | ||
| ) |
There was a problem hiding this comment.
Question: I agree we shouldn't be linting files in the build dir but I'm curious why it wasn't an issue before now. Is the upgraded version of Dokka now generating .kt files in build?
There was a problem hiding this comment.
I was running into an issue but it seems unreproducible now, I'll remove this
There was a problem hiding this comment.
Ran into the error again 😮💨, it's caused by some DSL accessors that are auto-generated
/Users/lauzmata/smithy-kotlin/buildSrc/build/generated-sources/kotlin-dsl-accessors/kotlin/gradle/kotlin/dsl/accessors/_15f47bddd2e04434c8db9f87ceea0581/DokkaHtmlGeneratorRuntimeConfigurationAccessors.kt:204:44: Function name should start with a lowercase letter (except factory methods) and use camel case (cannot be auto-corrected) (standard:function-naming)
/Users/lauzmata/smithy-kotlin/buildSrc/build/generated-sources/kotlin-dsl-accessors/kotlin/gradle/kotlin/dsl/accessors/_15f47bddd2e04434c8db9f87ceea0581/DokkaHtmlGeneratorRuntimeConfigurationAccessors.kt:32:10: Package name must not contain underscore (cannot be auto-corrected) (standard:package-name)
Summary error count (descending) by rule:
standard:function-naming: 274
standard:package-name: 102
standard:class-naming: 12
| alias(libs.plugins.kotlin.jvm) | ||
| id(libs.plugins.kotlin.jvm.get().pluginId) |
There was a problem hiding this comment.
Question: Why did this need to change? (applies in multiple build files)
There was a problem hiding this comment.
I was getting an error:
* Where:
Build file '/Users/lauzmata/smithy-kotlin/build.gradle.kts' line: 20
* What went wrong:
Error resolving plugin [id: 'org.jetbrains.kotlin.multiplatform', version: '2.1.0', apply: false]
> The request for this plugin could not be satisfied because the plugin is already on the classpath with an unknown version, so compatibility cannot be checked.
This comment has been minimized.
This comment has been minimized.
Affected ArtifactsNo artifacts changed size |
Issue #
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.