Added the ability to set the base name, version, classifier and build destination for multi-version configuration and improved support for Kotlin DSL #57
Conversation
…estinationDirectory` to the multiversion configurations and changed `inputJars` to `Property<FileCollection>` instead of `FileCollection` to add proper support for Kotlin DSL
WalkthroughThe changes update the configuration and usage of multiversion build properties in the Forgix Gradle plugin. Changes
Sequence Diagram(s)sequenceDiagram
participant GradleBuild
participant ForgixConfiguration
participant MultiversionConfiguration
participant MergeVersionsTask
GradleBuild->>ForgixConfiguration: Configure plugin
ForgixConfiguration->>MultiversionConfiguration: Instantiate with rootProject
MultiversionConfiguration-->>ForgixConfiguration: Provide archive and directory properties
GradleBuild->>MergeVersionsTask: Execute task
MergeVersionsTask->>MultiversionConfiguration: Retrieve archive and inputJar properties
MergeVersionsTask->>MergeVersionsTask: Perform merge using provided properties
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/io/github/pacifistmc/forgix/plugin/configurations/ForgixConfiguration.java(2 hunks)src/main/java/io/github/pacifistmc/forgix/plugin/tasks/MergeVersionsTask.java(2 hunks)
🔇 Additional comments (4)
src/main/java/io/github/pacifistmc/forgix/plugin/configurations/ForgixConfiguration.java (3)
281-281: LGTM: Correctly passes rootProject to MultiversionConfiguration constructor.This change enables the MultiversionConfiguration to access project information for setting up default conventions.
290-307: Well-implemented property-based configuration expansion.The addition of new archive-related properties and the change from
FileCollectiontoProperty<FileCollection>forinputJarscorrectly addresses the Kotlin DSL compatibility issues mentioned in the PR objectives. All properties are properly initialized using the ObjectFactory.
309-327: Excellent property conventions and Kotlin DSL compatibility fix.The getter methods provide sensible default conventions:
archiveBaseNamedefaults to project namearchiveClassifierdefaults to "multi" (appropriate for multiversion jars)archiveVersiondefaults to project versiondestinationDirectoryuses a dedicated subdirectory under buildThe change of
inputJarsto returnProperty<FileCollection>resolves the Kotlin DSL read-only issue mentioned in the PR objectives.src/main/java/io/github/pacifistmc/forgix/plugin/tasks/MergeVersionsTask.java (1)
45-47: Minor formatting improvement in exception message.The reformatting of the exception message improves readability while maintaining the same informative content.
src/main/java/io/github/pacifistmc/forgix/plugin/tasks/MergeVersionsTask.java
Outdated
Show resolved
Hide resolved
|
Sorry, I've been really busy recently. So it'll take me a while before I can get to this. Lazy evaluation is required for configuration caching |
|
Sure i can do that. |
I was writing a new template for future projects, and for testing purposes decided to try Forgix. I tried to get multi-version to work knowing it was experimental, and after a long time of troubleshooting, i found out why it didn't work. It seems you cant set the
inputJarsvariable in Kotlin DSL because it interprets the variable as read only. To fix this i changed the variable type fromFileCollectiontoProperty<FileCollection>which allows Kotlin DSL to modify the variable.I also found it annoying that the multi-version jar used the Forgix config's base name, version, classifier and build destination, so i decided to also add the ability to change those independent of the Forgix config.
I did this because when i build and merge, the jar names will contain both the mod version and the minecraft version, which i dont want the multi-version jar to have, it should only contain the mod version.
I also decided to add the ability to change the base name, classifier and build destination while i was at it.
I don't have much experience with making Gradle plugins, so if anything is wrong feel free to correct it. I also originally thought of making an issue, but since i already fixed the issues myself i decided to just open a pull request.
Summary by CodeRabbit