Skip to content

Migrate to DGPv2 #3005

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Migrate to DGPv2 #3005

wants to merge 4 commits into from

Conversation

whyoleg
Copy link
Contributor

@whyoleg whyoleg commented May 16, 2025

Migrate to new Dokka Gradle Plugin (migration guide)

Because of the Gradle task name change, the kotlinlang deploy configuration should also be updated.
Related: JetBrains/kotlin-web-site#4851

}
dokka.dokkaSourceSets.configureEach {
externalDocumentationLinks.register("kotlinx-io") {
url("https://kotlinlang.org/api/kotlinx-io/")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

JIC: old URL is not accessible anymore

Copy link
Member

Choose a reason for hiding this comment

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

Local build gives me Failed to download package-list from https://kotlinlang.org/api/kotlinx-io/package-list, this might suggest that remote resource is not available, module is empty or dokka output got corrupted, and Source/Sink declarations in the doc are not linked, despite URL being available

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, that looks strange, local build works fine on my side.
I can make it use the local package list (downloaded), as it was before for okio. The only case when the package list will become outdated is when a package is added or removed, so probably it should not be a big deal here to just use the local package list.
WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added local package lists for both kotlinx-io and okio, so that the build will be less dependent on network failures.

JIC: Dokka by default still relies on downloading the package list of the stdlib.

@@ -52,11 +52,4 @@ kotlin {
}
}

// This task should be disabled because of no need to build and publish intermediate JsWasm sourceset
tasks.whenTaskAdded {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This workaround is not needed, locally build and publish tasks works find without it, but it caused some issues locally at my side.
I can revert it, if you know the reason why it's needed

Copy link
Member

Choose a reason for hiding this comment

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

cc @igoriakovlev maybe this workaround is not needed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JIC, I've now additionally checked, and ./gradlew compileJsWasmMainKotlinMetadata executed successfully, so we don't need this hack anymore.
I believe initially, this source set contained some declarations that were available in JS and wasm-js targets, but without a shared declaration, as there is no shared source set in the stdlib for JS and wasm-js.
But I don't see anything like this now.


sourceLink {
localDirectory.set(rootDir)
perPackageOption {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, there were multiple perPackageOption - most of them are obsolete or separate configurations for internal package. So, obsolete were removed and internal are handled by one regex now

Copy link
Member

Choose a reason for hiding this comment

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

Are @Deprecated declarations skipped now by default?

Copy link
Contributor Author

@whyoleg whyoleg Jun 16, 2025

Choose a reason for hiding this comment

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

Nope, only HIDDEN ones. Looks like I missed the flag during refactoring
Is it desirable behavior to skip deprecated with WARNING and ERROR declarations in all modules?

Copy link
Contributor Author

@whyoleg whyoleg Jun 25, 2025

Choose a reason for hiding this comment

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

I've brought back skipDeprecated = true

@whyoleg whyoleg requested review from sandwwraith and removed request for sandwwraith June 25, 2025 11:27
localDirectory.set(rootDir)
perPackageOption {
matchingRegex = ".*\\.internal(\\..*)?"
suppress = true
Copy link
Member

Choose a reason for hiding this comment

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

reportUndocumented = false ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no need, as suppressed packages are skipped during analysis, and so no warnings will be present for them

@whyoleg whyoleg requested a review from sandwwraith June 26, 2025 12:11
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