-
Notifications
You must be signed in to change notification settings - Fork 408
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
Fix relocation exclusion for file patterns like kotlin/kotlin.kotlin_builtins
#1313
Conversation
Goooler
commented
Mar 6, 2025
•
edited
Loading
edited
- Closes Relocation Duplicates kotlin.kotlin_builtins #295.
- Closes Exclude in relocations disregarded for kotlin_builtins #562.
- Closes [Question] How to avoid string in classes being relocated #884.
- CHANGELOG's "Unreleased" section has been updated, if applicable.
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.
Code Review
The pull request introduces a new test case preserveKotlinBuiltins
to verify that Kotlin built-in files are correctly preserved during relocation. The test creates a Kotlin JAR, adds a Kotlin built-ins file, and then uses the shadowJar
task to relocate the Kotlin package while excluding the built-ins file. The test then asserts that the built-ins file is present in the output JAR and not relocated. Overall, the test seems well-structured and addresses the intended functionality.
Merge Readiness
The code changes introduce a new test case that enhances the project's testing suite. Given the absence of any identified critical or high severity issues, the pull request appears to be in good shape. I am unable to directly approve the pull request, and recommend that others review and approve this code before merging. However, I believe it is ready for merging once any minor feedback is addressed.
preserveKotlinBuiltins
kotlin/kotlin.kotlin_builtins
kotlin/kotlin.kotlin_builtins
kotlin/kotlin.kotlin_builtins
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.
PR Overview
This PR updates the changelog to document a fix for the relocation exclusion for file patterns like "kotlin/kotlin.kotlin_builtins".
- Clarifies the fixed issue in the changelog.
- Adds a new "Fixed" section detailing the change.
Reviewed Changes
File | Description |
---|---|
docs/changes/README.md | Added a changelog entry for the fix. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
d17d04f
to
5c6b5ce
Compare