feat: enhance plugin file generation with JSON output and task improvements#211
feat: enhance plugin file generation with JSON output and task improvements#211twisti-dev merged 2 commits intoversion/1.21.11from
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad100076e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @get:OutputDirectory | ||
| abstract val outputDirectory: DirectoryProperty | ||
| abstract val outputFile: RegularFileProperty |
There was a problem hiding this comment.
The task now exposes outputFile as a RegularFileProperty, but it is still annotated with @OutputDirectory. Gradle validates output property types, so pairing @OutputDirectory with a regular file can cause configuration-time errors (e.g., “Expected a directory but found a file”) or incorrect up-to-date/caching behavior when the task writes a single file. This will surface whenever the task runs on a clean build or with configuration caching enabled. Use @OutputFile for a single file output to match the property type and task behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR refactors the Gradle plugin’s plugin-file generation to improve cacheability by switching from domain object inputs to direct JSON inputs, and modernizes related task/container setup in the gradle plugin module.
Changes:
- Refactor
GeneratePluginFileto take a JSON string input and write to a single output file; mark the task cacheable. - Update
CommonSurfPluginWithPluginFileto pre-serialize plugin file objects to JSON and wire a single output file. - Mark
GenerateLibrariesLoaderTaskcacheable and modernizeVelocityPluginFiledependency container creation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| surf-api-gradle-plugin/src/main/kotlin/dev/slne/surf/surfapi/gradle/platform/common/CommonSurfPluginWithPluginFile.kt | Switches task wiring to pass JSON and a single output file to plugin-file generation. |
| surf-api-gradle-plugin/src/main/kotlin/dev/slne/surf/surfapi/gradle/generators/pluginfiles/VelocityPluginFile.kt | Uses project.objects.domainObjectContainer for dependency container creation. |
| surf-api-gradle-plugin/src/main/kotlin/dev/slne/surf/surfapi/gradle/generators/LibrariesLoaderGenerator.kt | Marks loader generation task as cacheable and cleans output before generating. |
| surf-api-gradle-plugin/src/main/kotlin/dev/slne/surf/surfapi/gradle/generators/GeneratePluginFile.kt | Implements JSON-string based generation and changes output to a single file. |
|
|
||
| @get:OutputDirectory | ||
| abstract val outputDirectory: DirectoryProperty | ||
| abstract val outputFile: RegularFileProperty | ||
|
|
There was a problem hiding this comment.
outputFile is a RegularFileProperty but is annotated as @OutputDirectory. Gradle task property validation will treat this as an invalid output type and may fail the build / disable caching. Change the annotation (and import) to @OutputFile (with @get:OutputFile).
| @get:Input | ||
| abstract val fileName: Property<String> | ||
|
|
||
| @get:Nested | ||
| abstract val pluginFile: Property<CommonPluginFile> | ||
| @get:Input | ||
| abstract val pluginFileJson: Property<String> |
There was a problem hiding this comment.
fileName is still declared as an @Input, but the task no longer uses it (the output path is fully determined by outputFile). This adds unnecessary configuration and affects the task cache key; consider removing fileName entirely or using it to derive outputFile inside the task configuration.
| group = "surf-api" | ||
|
|
||
| fileName.set(pluginFileName) | ||
|
|
||
| outputFile.set(generatedResourcesDirectory.map { it.file(pluginFileName) }) |
There was a problem hiding this comment.
fileName is configured on GeneratePluginFile, but the task implementation no longer uses it (it writes to outputFile). Once fileName is removed/repurposed in the task, this configuration should be dropped to avoid misleading inputs.
1 participant
This pull request refactors the Gradle plugin file generation logic to improve task cacheability and simplify serialization. The main changes involve switching from domain object serialization to direct JSON handling, updating task input/output properties, and ensuring tasks are cacheable. Additionally, dependency container creation and task registration have been modernized.
Plugin file generation refactoring:
GeneratePluginFileto use a JSON string property (pluginFileJson) as input and a single file property (outputFile) as output, replacing domain object serialization and directory output. Task is now annotated as@CacheableTaskfor improved build caching.CommonSurfPluginWithPluginFileto serialize plugin file objects to JSON before passing to the task, and to set the output file property accordingly.Task and dependency container improvements:
GenerateLibrariesLoaderTaskwith@CacheableTaskand added logic to delete the output directory before generation for correctness and cacheability. [1] [2]VelocityPluginFileto useproject.objects.domainObjectContainerinstead of the deprecatedproject.container.Import and cleanup adjustments: