Skip to content

Note the mixed usages of ResourceTransformer and DuplicatesStrategy#1349

Merged
Goooler merged 2 commits intomainfrom
g/20250315/not-dup-strategy-priority
Mar 15, 2025
Merged

Note the mixed usages of ResourceTransformer and DuplicatesStrategy#1349
Goooler merged 2 commits intomainfrom
g/20250315/not-dup-strategy-priority

Conversation

@Goooler
Copy link
Copy Markdown
Member

@Goooler Goooler commented Mar 15, 2025 •
edited
Loading

@Goooler Goooler requested a review from Copilot March 15, 2025 00:18
gemini-code-assist[bot]
gemini-code-assist bot reviewed Mar 15, 2025
View reviewed changes
Copy link
Copy Markdown
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.

Copilot AI reviewed Mar 15, 2025
View reviewed changes
Copy link
Copy Markdown
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
@Goooler Goooler mentioned this pull request Jul 31, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@Goooler