-
Notifications
You must be signed in to change notification settings - Fork 471
Dokka Gradle plugin doc update #4354
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: master
Are you sure you want to change the base?
Conversation
whyoleg
left a 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.
First round of review :)
docs/topics/runners/dokka-gradle.md
Outdated
| tasks.dokkaHtmlPartial { | ||
| outputDirectory.set(layout.buildDirectory.dir("docs/partial")) | ||
| } | ||
| Follow the next steps to configure your multi-module project without convention plugins. |
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 get this part. It should be the next steps without convention plugins, but then we start describing how to create a convention plugin.
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.
"Without" means that require one, that's why we have the instructions to create one. I've updated it, hopefully it is clearer.
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.
Ah, it's about projects that don't have convention plugins yet, and we show how to add them. Thanks.
But then the two sections with/without are not so different. In the first one we describe the project structure and explain how to do it. And in the second it's almost the same, but it is described in Gradle documentation that we refer to.
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.
@adam-enko I remember writing these instructions with content you provided. Any idea about how to better differenciate Direct configuration in multi-project builds requiring convention plugins vs Multi-project builds with convention plugins sections?
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.
Yeah, the steps seem a bit muddled up.
We want to describe two ways of configuring Dokka in multimodule projects:
- Manually copy-paste the
dokka {}config to each subproject. - (preferred) Use a convention plugin.
So the section Direct configuration in multi-project builds requiring convention plugins seems strange. It should be Shared configuration using convention plugins, which should describe how to set up buildSrc (if necessary, i.e. the project doesn't already have convention plugins), and then describe an example convention plugin and how to apply it.
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 sent you a Slack message to clarify it (:
729484a to
dbfcbe1
Compare
whyoleg
left a 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.
Nice!
| {style="note"} | ||
|
|
||
| * For [Gradle](dokka-gradle.md#generate-documentation), run the following tasks: | ||
| * `dokkaGenerate` to generate documentation in [all available formats based on the applied plugins](dokka-gradle.md#configure-documentation-output-format). |
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.
At this point, I've realized that we might have done something wrong with DGPv2 tasks API...
By default, for the HTML format we have the following tasks:
dokkaGenerate- shown in IDE, run's alldokkaGenerate*dokkaGenerateHtml- shown in IDE, run'sdokkaGeneratePublicationHtml(just an alias)dokkaGenerateModuleHtml- hidden in IDE, internal taskdokkaGeneratePublicationHtml- hidden in IDE, REAL task, which runs Dokka, and which users might need to depend on in code (for outputs)
Now comes the question: how that happened that we have dokkaGenerateHtml and dokkaGeneratePublicationHtml which are just aliases, but one users will see in IDE, but the other one, they need to use in code?
@adam-enko, am I missing something?
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.
sorry, I don't get the question, could you rephrase it?
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.
The question is, did I correctly describe the current situation with tasks?
However, what I wanted to highlight is that there should be only one task to run the Dokka generation for a specific format. We have two of them (dokkaGenerateHtml and dokkaGeneratePublicationHtml) which do the same thing, but for some reason, we show dokkaGenerateHtml in the IDE (dokka group), but users need to use dokkaGeneratePublicationHtml in Gradle scripts if they want to get their output. There are no other differences between those tasks.
It appears that, at some point, we simply missed this... And as DGPv2 is now stable, we can't change it.
What I propose, then, is that we consider dropping the usage of dokkaGenerateHtml in documentation/errors/etc and hiding it in the IDE, while using dokkaGeneratePublicationHtml everywhere, thereby showing it in the IDE. E.g., soft deprecation of dokkaGenerateHtml task.
We can't do otherwise, as users might already rely on the fact that dokkaGeneratePublicationHtml has outputs.
It's a bit sad that it happened after a stable release, but it feels better than explaining the differences.
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.
Okay, thanks, I see your point.
I don't think it's that much of a mistake. It's just a different approach to Gradle plugin development.
Speaking generally, it's a good idea to keep the API surface small, and part of that is only exposing lifecycle tasks. It helps give us some flexibility in the future, in case we need to modify the actual worker tasks (e.g. move them from DGP into KGP). It also helps guide users away from task-based configuration and towards using dokka {}.
That said, in this instance, the difference between the two tasks is quite subtle. It would make sense to update dokkaGenerateHtml to be the user-facing task that provides the output (e.g. it can be used in a JAR task). We can deprecate or de-prioritise dokkaGeneratePublicationHtml if necessary. There are a few paths for migration.
Another thought: As far as I can tell, the only reason dokkaGeneratePublicationHtml needs to be documented is because DGP doesn't have a dokkaGenerateHtmlJar. If we had that, then we could expose that as the public task and keep the other tasks as 'soft' internal?
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 think that dokkaGenerateHtmlJar will fully solve the problem, as dokkaGeneratePublicationHtml might be used for integration not only with jars, but with any other Gradle plugin or external tools. One small example, to just show the idea (leaving aside that it could be done in a lot of other ways in Gradle).
But, overall, I understand the idea better now (e.g., only exposing lifecycle tasks.). I still think it might be too much in that case, as we expose dokkaGeneratePublicationHtml to users (with all its inputs/outputs), so we will not be able to change it without breaking API/behaviour compatibility.
It would make sense to update dokkaGenerateHtml to be the user-facing task that provides the output
Can we do this safely? I mean, is it fine that both tasks will provide the same outputs?
All in all, I would really avoid changing the behavior at this point and mostly concentrate on how we document things. That's why I suggested not mentioning dokkaGenerateHtml in the docs. And looks like we don't mention them, so it should be fine as it is now. :)
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.
Avoiding documenting dokkaGenerateHtml is fine with me, although given that dokkaGenerateHtml is visible in IJ under the dokka task group I think it would make sense to document it now. We can have another pass to make the tasks more consistent in the future.
Can we do this safely? I mean, is it fine that both tasks will provide the same outputs?
Basically, yes. In theory some projects might have some unusual configuration that causes problems, but I think it's very unlikely.
Another option: we expose the task via the dokka {} project extension. So instead of tasks.dokkaGeneratePublicationHtml we direct users to use dokka.html.publicationTask.
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.
although given that dokkaGenerateHtml is visible in IJ under the dokka task group I think it would make sense to document it now.
Yes, we can do this, though I'm a bit concerned about how/where to document this.
Should we just say that "it's also possible to use dokkaGenerateHtml, as it's just an alias/lifecycle task"? Probably, but for me it's a bit confusing: why do I need to know this when I'm reading the docs?
Gradle already has numerous ways to accomplish tasks, and we don't need to add to this even more.
As for now, I do see two main user flows:
- user migrates or adds Dokka to the project -> they will see the task named
dokkaGeneratePublicationHtmlin the documentation, and so they will use it. - user who migrated and wants to run Dokka -> they will open the IDE, see
dokkaGenerateanddokkaGenerateHtml, and will just click on the correct task. (or will grepgradle tasksoutput and do the same)
All in all, I think adding the alias task name (dokkaGenerateHtml) to the docs is fine; I'm just concerned it may be confusing. Maybe @AlejandraPedroza has some ideas on how we could mention it, without confusing readers. Both options are fine for me at this point.
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 added the following note in the appropriate places, just to avoid surprises and let users know they may find this task when using IntelliJ IDEA:
If you're using IntelliJ IDEA, you may see the
dokkaGenerateHtmlGradle task.
This task is simply an alias ofdokkaGeneratePublicationHtml. Both tasks perform exactly the same operation.
This PR updates all the documents relevant to Dokka Gradle plugin v2 # Conflicts: # docs/topics/dokka-get-started.md # docs/topics/dokka-plugins.md # docs/topics/formats/dokka-html.md # docs/topics/formats/dokka-markdown.md
127f39b to
0f1dc64
Compare
whyoleg
left a 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.
Thank you! I left just some minor comments.
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.
If this page is deleted then will all external links to it 404?
I vaguely remember there's a way in Writerside to create 'dead' links? So there's no content, but the URL is still active and we can redirect users to somewhere more helpful?
But, I don't think it's a big deal. Just asking in case there's an easy-to-enable option.
| When documenting multi-project builds, you don't need to apply the plugin explicitly to every subproject you want to document. | ||
| Instead, | ||
| Dokka expects you to share configuration across subprojects using convention plugins or manual configuration per subproject. | ||
| For more information, see | ||
| how to configure [single-project](#single-project-configuration) and [multi-project](#multi-project-configuration) builds. |
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.
| When documenting multi-project builds, you don't need to apply the plugin explicitly to every subproject you want to document. | |
| Instead, | |
| Dokka expects you to share configuration across subprojects using convention plugins or manual configuration per subproject. | |
| For more information, see | |
| how to configure [single-project](#single-project-configuration) and [multi-project](#multi-project-configuration) builds. | |
| When documenting multi-project builds, you need to apply the plugin explicitly to every subproject you want to document. | |
| Dokka can be configured directly in each subproject, or shared configuration can be shared using a convention plugins. | |
| For more information, see | |
| how to configure [single-project](#single-project-configuration) and [multi-project](#multi-project-configuration) builds. |
Two suggestions:
- "you don't need to apply the plugin explicitly" - Dokka must be applied explicitly.
- Tidy the phrasing about how Dokka can be configured. (This is an optional suggestion - feel free to reject to re-write).
This PR updates all the documents relevant to Dokka Gradle plugin v2, now that with Dokka 2.1.0, the DGP v2 is enabled by default: