Skip to content

Simplify Dokka source link configuration #3969

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

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

whyoleg
Copy link
Contributor

@whyoleg whyoleg commented Dec 4, 2023

Based on discussion: Kotlin/kotlinx-datetime#323 (comment)

Checked locally - content is identical for both approaches

@whyoleg whyoleg requested a review from qwwdfsad December 4, 2023 19:27
@whyoleg whyoleg self-assigned this Dec 4, 2023
Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

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

Thank you for remembering to cleanup the code in other repositories! After seeing the discussion in kotlinx-datetime, I went here to adapt the fix and was glad that someone's already done it!

I compared the HTML files and did find a difference, but it doesn't seem meaningful. I don't even understand it. In several files, <code class="org.jetbrains.dokka.pages.commenttable@2c36c6b1 lang-kotlin"> was replaced with <code class="org.jetbrains.dokka.pages.commenttable@37650479 lang-kotlin">. No idea what this means, but thought I should bring it up just in case.

@whyoleg
Copy link
Contributor Author

whyoleg commented Dec 5, 2023

Yeah, I've also compared the content locally. This is expected, but unobvious change :)
This is coming from calling Style::toString() somewhere in Dokka code...
This specific string is for CommentTable style. And as it's an object we have hashCode in toString.

Overall this specific Style object should not be present in HTML at all, but just used for some logic branching...
So, it's safe change, I believe some time in the future we will cleanup such things.

@whyoleg whyoleg force-pushed the whyoleg-sourcelinks branch from 3eea811 to 2e7f767 Compare December 5, 2023 12:26
@whyoleg
Copy link
Contributor Author

whyoleg commented Dec 5, 2023

Sorry, accidentally pushed another changes to wrong branch - force pushed back to previous commit

@dkhalanskyjb dkhalanskyjb merged commit 5251db6 into Kotlin:master Dec 5, 2023
@whyoleg whyoleg deleted the whyoleg-sourcelinks branch December 5, 2023 12:44
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