Skip to content

Conversation

@Goooler
Copy link
Member

@Goooler Goooler commented Mar 15, 2025

@Goooler Goooler requested a review from Copilot March 15, 2025 00:18
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request adds documentation to explain the precedence of duplicatesStrategy over transforming and relocating in the ShadowJar plugin. The documentation provides examples in both Kotlin and Groovy. Overall, the changes are clear and helpful for users of the plugin.

Merge Readiness

The documentation changes are clear and accurate. The pull request is ready to be merged. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the documentation regarding the interaction between duplicatesStrategy and resource transforming in the merging process to improve clarity.

  • Updated notes to highlight precedence of duplicatesStrategy over resource transformations.
  • Added examples in both Kotlin and Groovy to illustrate potential issues with mixing duplicatesStrategy and ResourceTransformer.
Comments suppressed due to low confidence (2)

docs/configuration/merging/README.md:401

  • Consider clarifying the term 'transforming' by consistently using 'ResourceTransformer' (or another precise term) to avoid ambiguity with ServiceFileTransformer.
**NOTE:** The `duplicatesStrategy` takes precedence over transforming and relocating. If you mix the usages of

docs/configuration/merging/README.md:422

  • Clarify the intended behavior of ServiceFileTransformer when used with duplicatesStrategy. Rephrase the sentence to clearly state whether the exclusion of duplicated service files is an intentional feature or a limitation.
The `ServiceFileTransformer` will not work as expected because the `duplicatesStrategy` will exclude the duplicated 

@Goooler Goooler changed the title Note the mixed usages of transforming and DuplicatesStrategy Note the mixed usages of transforming and DuplicatesStrategy Mar 15, 2025
@Goooler Goooler changed the title Note the mixed usages of transforming and DuplicatesStrategy Note the mixed usages of ResourceTransformer and DuplicatesStrategy Mar 15, 2025
@Goooler Goooler enabled auto-merge (squash) March 15, 2025 00:25
@Goooler Goooler merged commit 8464ed0 into main Mar 15, 2025
7 checks passed
@Goooler Goooler deleted the g/20250315/not-dup-strategy-priority branch March 15, 2025 00:30
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.

mergeServiceFiles() does not work as expected when using DuplicatesStrategy.EXCLUDE

2 participants