-
Notifications
You must be signed in to change notification settings - Fork 25
feat(docs-generator): add Dokka documentation generation templates #92
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
base: dev
Are you sure you want to change the base?
feat(docs-generator): add Dokka documentation generation templates #92
Conversation
1713745 to
ce5909c
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
@biplab1 this will fix the issue also pull the latest changes and rebase this branch
| // Dokka for documentation | ||
| register("dokkaConvention") { | ||
| id = "org.convention.dokka.plugin" | ||
| implementationClass = "org.convention.DokkaConventionPlugin" |
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.
change it to DokkaConventionPlugin
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.
Changed it to DokkaConventionPlugin
core-base/common/build.gradle.kts
Outdated
| } | ||
| } | ||
|
|
||
| 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.
You won't need to call this block as you're configuring this on the convention plugin using this method(configureDokkaConvention)
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.
Forgot to remove that block. Removed them now.
core/database/build.gradle.kts
Outdated
| alias(libs.plugins.kotlin.serialization) | ||
| alias(libs.plugins.kotlin.parcelize) | ||
| alias(libs.plugins.mifos.kmp.room) | ||
| alias(libs.plugins.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.
Also, you no need to apply this plugin on (core,core-base, feature) modules as this applied on KMP convention plugin so it will applied automatically, but you need to apply this on the application modules such as cmp-android, cmp-desktop etc if the kmp plugin isn't applied on there and need to manually set the module name using the dokka block like below.
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.
I have removed the dokka plugin in modules where KMP convention is present and applied in those where KMP convention is absent. Module name is not required to be set for modules other than the once inside core and core-base since those have same names which needs to be uniquely defined for the navigation panel in dokka to be correctly displayed.
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.
I don't understand brother, are you asking about some issues or just updating me,
If you getting issue regarding the core,core-base module names as few might present on both directory then you need update this method(configureDokkaConvention) accordingly will solve the issue, no need to set the package name explicitly on those 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.
And as I can see that method also handle it properly as it remove the colon(:) and replace with hyphen(-) so for exam the (core: analytics) will be core-analytics and the (core-base:analytics) would be core-base-analytics and if you want to configure it in other format then update that method accordingly @biplab1
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.
Module name is not required to be set for modules other than the once inside core and core-base since those have same names which needs to be uniquely defined for the navigation panel in dokka to be correctly displayed.
I meant that we don’t need to change the module names for cmp-android, cmp-desktop, cmp-web, etc. Also, it was a statement, not a question, since it lacks grammatical structuring like that of a question and doesn’t include a question mark.
2edabb7 to
bf783a8
Compare
bf783a8 to
0e1066e
Compare
Jira ticket: KMPPT-81
Summary of Changes: