Skip to content
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

update: DGPV2 migration guide #4025

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

AlejandraPedroza
Copy link
Contributor

@AlejandraPedroza AlejandraPedroza commented Feb 10, 2025

This PR contains updates for the DGP V2 Migration guide.

The additions to the Migration guide are:

  • Add context for cases when people are not using convention plugins in their multi-module build.
  • Clarify a bit earlier that the formats you obtain now require adding more dependencies (if you want javadoc)
  • externalDocumentationLinks has changed API and it's missing in migration guide
  • Document Output directory for additional files
  • Document how to configure Dokka Plugins
  • Document how to configure custom Dokka Plugins
  • Document sourceLink
  • Highlight what are migration helpers in migration guide
  • Document the change of directory in DGPv2

Additionally, there are some fixes in the code format and indentation. Plus, the content within "Adjust configuration options" is now broken into single subsections instead of bullet points.

KT-72053 [Dokka] DGP v2 migration guide feedback

@AlejandraPedroza AlejandraPedroza force-pushed the DGPV2-update-migration-guide branch from f4b0526 to fb10fbd Compare February 10, 2025 14:11
@AlejandraPedroza AlejandraPedroza changed the title This PR contains updates for the DGP V2 Migration guide. update: DGPV2 migration guide Feb 11, 2025
Copy link
Collaborator

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

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

Nice! Thank you!
There are some small comments from me, but overall looks good!

@AlejandraPedroza AlejandraPedroza force-pushed the DGPV2-update-migration-guide branch from ed43b97 to dd02f2e Compare February 25, 2025 14:44
Copy link
Collaborator

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

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

found some more minor nitpicks :)

@daniCsorbaJB daniCsorbaJB self-assigned this Mar 7, 2025
@adam-enko adam-enko added the documentation An issue/PR related to Dokka's external or internal documentation label Mar 10, 2025
Copy link

@daniCsorbaJB daniCsorbaJB 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 the changes - I left a couple of comments, please let me know if you have any questions


2. In the project's `gradle.properties` file, set the following opt-in flag with helpers to activate the new plugin version:
> By default, DGP v2 generates HTML documentation. To generate Javadoc or both HTML and Javadoc formats,

Choose a reason for hiding this comment

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

Hmm 🤔
What do you think of this version?
I assume HTML plugin is already included if that's the default version:

By default, DGP v2 generates documentation in HTML format.
To generate Javadoc or both HTML and Javadoc, add the Javadoc plugin.

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 applied your suggestion in the first part of the sentence. However, I kept it as "appropriate plugins" because the HTML and the Javadoc are different ones.

```
### Enable migration helpers

In the project's `gradle.properties` file, set the following opt-in flag to activate the new plugin version with helpers:

Choose a reason for hiding this comment

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

I don't think we use flag in our docs (this might be an exception?) - would Gradle property work here?

>
{style="tip"}

This flag activates the DGP v2 plugin with migration helpers, which prevent compilation errors when build scripts reference

Choose a reason for hiding this comment

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

Suggested change
This flag activates the DGP v2 plugin with migration helpers, which prevent compilation errors when build scripts reference
This property activates the DGP v2 plugin with migration helpers, which prevent compilation errors when build scripts reference

Choose a reason for hiding this comment

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

(Suggestion)
Could we break this into two sentences?
This property activates the DGP v2 plugin with migration helpers.
These helpers prevent compilation errors when build scripts reference tasks from...

When DGP aggregates modules, each subproject has its own subdirectory within the aggregated docs.

In DGP v2, the aggregation mechanism has been updated to better align with Gradle conventions.
DGP v2 now preserves the full subproject directory to prevent conflicts when aggregating

Choose a reason for hiding this comment

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

Suggested change
DGP v2 now preserves the full subproject directory to prevent conflicts when aggregating
It preserves the full subproject directory to prevent conflicts when aggregating

Copy link
Contributor Author

@AlejandraPedroza AlejandraPedroza Mar 18, 2025

Choose a reason for hiding this comment

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

As the last noun mentioned is "Gradle conventions" and "mechanism" is before that, I'll leave DGP v2 instead of "it" for better clarity.


In DGP v1, aggregated documentation was placed in a collapsed directory structure.
For example, given a project with an aggregation in `:turbo-lib` and a nested subproject `:turbo-lib:maths`,
the generated documentation would be placed under:

Choose a reason for hiding this comment

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

Suggested change
the generated documentation would be placed under:
the generated documentation was placed under:

turbo-lib/build/dokka/html/maths/
```

New aggregation directory:

Choose a reason for hiding this comment

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

like before and for all mentions - I think it would be more future-proof to say :

DGP v1 aggregation directory:

DGP v2 aggregation directory:

instead of old and new

turbo-lib/build/dokka/html/turbo-lib/maths/
```

This change prevents subprojects with the same name from clashing. However, because the directory has changed, external links

Choose a reason for hiding this comment

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

Suggested change
This change prevents subprojects with the same name from clashing. However, because the directory has changed, external links
This change prevents subprojects with the same name from clashing. However, since the directory structure has changed, external links

```

This change prevents subprojects with the same name from clashing. However, because the directory has changed, external links
may become outdated and cause `404` errors.

Choose a reason for hiding this comment

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

Suggested change
may become outdated and cause `404` errors.
may become outdated, potentially causing `404` errors.

@AlejandraPedroza AlejandraPedroza force-pushed the DGPV2-update-migration-guide branch from ff57df6 to fc203ca Compare March 21, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An issue/PR related to Dokka's external or internal documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants