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

Remove configuration of transitive Dokka Engine dependencies #3794

Merged

Conversation

adam-enko
Copy link
Member

@adam-enko adam-enko commented Sep 10, 2024

Remove unnecessary configuration of Dokka Engine transitive dependencies.

DGP should only depend on Dokka Engine, and not prescriptively control what dependencies to declare.

KT-71382

@adam-enko adam-enko added the runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin label Sep 10, 2024
@adam-enko adam-enko added this to the Dokka 2.0.0 milestone Sep 10, 2024
@adam-enko adam-enko marked this pull request as ready for review September 12, 2024 15:14
@@ -134,8 +134,9 @@ abstract class DokkaFormatPlugin(
// https://github.com/gradle/gradle/issues/27435)
dependenciesContainer.resolutionStrategy.eachDependency {

Choose a reason for hiding this comment

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

FYI: Using .eachDependency to specify version could lead to Gradle complain. I've fixed similar issue inside kotlin.git. Proposed solution is to use constraints with required (or some other level) version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Constraints require a list of all Dokka dependencies, and I don't think we have this.

I think improving this is on Gradle to fix gradle/gradle#15089

* This value defaults to the current Dokka Gradle Plugin version, but can be overridden
* if you want to use a newer or older version of Dokka at runtime.
*/
val dokkaEngine: Property<String>
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not to move this on extension level and call just dokkaEngineVersion/engineVersion - similar to how KGP has compilerVersion when using build-tools-api?

Not sure we need Versions interface anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I'll move it to the top level.

The reason I wanted to keep it nested inside a versions {} block was to avoid having too many options in the top level of dokka {}. I think keeping the lower-level options nested helps indicate that they are more complicated and advanced. Although I agree that a versions {} block seems pretty pointless without other versions, and it's unlikely that we'll be adding other versions.

tbh I think the top level dokka {} DSL is pretty disorganised and could do with a review, but it's hard to re-organise it without becoming too disconnected from the underlying Dokka configuration.

Anyway, we can revisit this later if needed.

@adam-enko adam-enko force-pushed the adam/feat/KT-70336/remove-transitive-dependency-config branch 2 times, most recently from d9dd54b to ff1c959 Compare September 18, 2024 11:43
…ies.

DGP should only depend on Dokka Engine, and not prescriptively control what dependencies to declare.

KT-71382
@adam-enko adam-enko force-pushed the adam/feat/KT-70336/remove-transitive-dependency-config branch from ff1c959 to cc9ca6f Compare September 18, 2024 11:46
@adam-enko adam-enko merged commit e6ac5b8 into master Sep 18, 2024
14 checks passed
@adam-enko adam-enko deleted the adam/feat/KT-70336/remove-transitive-dependency-config branch September 18, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runner: Gradle plugin An issue/PR related to Dokka's Gradle plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants