Use jpenilla/resource-factory to generate plugin.yml file #66
Conversation
WalkthroughThis pull request removes the legacy plugin configuration API and replaces it with a new system based on Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Gradle
participant BukkitGradlePlugin
participant BukkitExtension
participant BukkitPluginYaml
participant ResourceFactoryPlugin
participant ParsePluginYamlTask
User->>Gradle: Apply BukkitGradlePlugin
Gradle->>BukkitGradlePlugin: apply()
BukkitGradlePlugin->>BukkitExtension: Create extension
BukkitGradlePlugin->>ResourceFactoryPlugin: Apply plugin
BukkitGradlePlugin->>BukkitPluginYaml: Register extension "plugin"
BukkitGradlePlugin->>Gradle: Register ParsePluginYamlTask
Gradle->>ParsePluginYamlTask: Execute (if plugin.yml exists)
ParsePluginYamlTask->>BukkitPluginYaml: Apply conventions from parsed YAML
Gradle->>ResourceFactoryPlugin: Generate or update plugin.yml
Assessment against linked issues
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. 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: 15
🔭 Outside diff range comments (1)
src/main/kotlin/server/DevServerPlugin.kt (1)
66-79: 💡 Verification agent🧩 Analysis chain
Verify Java toolchain functionality after refactoring
With the change in how the Java toolchain is accessed, it's important to ensure that the server still runs with the correct Java version configuration.
This change assumes that the
javaextension property is properly registered and accessible. Let's verify that it's being registered correctly:
🏁 Script executed:
#!/bin/bash # Check for the java extension property registration grep -r "val Project.java" src/main/kotlin/Length of output: 132
🏁 Script executed:
#!/bin/bash FILE=src/main/kotlin/server/DevServerPlugin.kt grep -n 'apply("java"' "$FILE" || echo "No direct java plugin apply found" grep -n "pluginManager.apply" "$FILE" || echo "No pluginManager.apply found" grep -n "plugins.apply" "$FILE" || echo "No plugins.apply found" grep -n "JavaPlugin" "$FILE" || echo "No JavaPlugin reference found"Length of output: 584
🏁 Script executed:
#!/bin/bash # Locate the DevServerPlugin implementation grep -R -n 'class DevServerPlugin' src/main/kotlin # Display the first 200 lines of the DevServerPlugin file for inspection FILE=$(grep -R -n 'class DevServerPlugin' src/main/kotlin | cut -d: -f1) echo "Inspecting $FILE" sed -n '1,200p' "$FILE"Length of output: 4574
Confirm Java plugin application in
BukkitGradlePluginWe need to ensure that the
javaextension (and thusjava.toolchain) is always available beforeconfigureDefaultJvmForServerruns.If the Java plugin is applied (directly or transitively) in your
ru.endlesscode.bukkitgradlesetup, thenjava.toolchainwill be present and the snippet is safe. Otherwise, you may need to apply the Java plugin inDevServerPlugin(or upstream) to guard against missingjavaextension.Please verify that the Java plugin is indeed applied before
DevServerPluginruns.
🏁 Script executed:
#!/bin/bash # Find where the main BukkitGradle plugin applies the Java plugin grep -R -n "apply<JavaPlugin" src/main/kotlin || echo "No direct apply<JavaPlugin> found" grep -R -n "plugins.apply(\"java\"" src/main/kotlin || echo "No plugins.apply(\"java\") found"Length of output: 247
Apply the Java plugin before accessing
java.toolchainCurrently,
Project.javais defined inextensions/Accessors.kt, but no part of the plugin code actually applies the Java plugin—sojava.toolchainmay not be registered at runtime. Please add the Java plugin inDevServerPlugin(or upstream) to guarantee the extension is present:• File:
src/main/kotlin/server/DevServerPlugin.kt
At the start ofapply(target: Project)insert:import org.gradle.api.plugins.JavaPlugin override fun apply(target: Project) { // Ensure JavaPlugin is applied so `java.toolchain` is available target.plugins.apply(JavaPlugin::class.java) project = target bukkit = project.bukkit … }• Verify that
import org.gradle.api.plugins.JavaPluginis added.
• Run a full build and confirmRunServertasks pick up the intended Java toolchain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
README.md(0 hunks)build.gradle.kts(1 hunks)src/main/kotlin/Bukkit.kt(2 hunks)src/main/kotlin/BukkitExtension.kt(2 hunks)src/main/kotlin/BukkitGradlePlugin.kt(2 hunks)src/main/kotlin/extensions/Accessors.kt(1 hunks)src/main/kotlin/extensions/Provider.kt(1 hunks)src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt(1 hunks)src/main/kotlin/plugin/PluginConfigurationPlugin.kt(0 hunks)src/main/kotlin/plugin/PluginYaml.kt(0 hunks)src/main/kotlin/plugin/PluginYamlFeature.kt(1 hunks)src/main/kotlin/plugin/extension/PluginConfiguration.kt(0 hunks)src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt(0 hunks)src/main/kotlin/plugin/task/MergePluginYaml.kt(0 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/main/kotlin/server/DevServerPlugin.kt(3 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/PluginSpecification.groovy(2 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy(0 hunks)
💤 Files with no reviewable changes (7)
- src/main/kotlin/plugin/extension/PluginConfiguration.kt
- src/main/kotlin/plugin/PluginYaml.kt
- src/main/kotlin/plugin/PluginConfigurationPlugin.kt
- README.md
- src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy
- src/main/kotlin/plugin/task/MergePluginYaml.kt
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/kotlin/Bukkit.kt (1)
src/main/kotlin/BukkitExtension.kt (2)
ReplaceWith(32-35)ReplaceWith(38-44)
src/main/kotlin/BukkitExtension.kt (2)
src/main/kotlin/plugin/PluginYamlFeature.kt (1)
plugin(123-125)src/main/kotlin/Bukkit.kt (1)
ReplaceWith(8-27)
🪛 detekt (1.23.7)
src/main/kotlin/plugin/task/ParsePluginYaml.kt
[warning] 46-46: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check
🔇 Additional comments (17)
build.gradle.kts (1)
32-32: Added dependency to xyz.jpenilla.resource-factory aligns with PR objectivesThe addition of the
resource-factoryplugin dependency supports the transition from custom plugin.yml generation to using jpenilla's resource-factory, which was the main goal of this PR.src/test/groovy/ru/endlesscode/bukkitgradle/PluginSpecification.groovy (2)
31-31: Added Java plugin to test setupThis change is consistent with the modification in BukkitGradlePluginSpec where the Java plugin now needs to be explicitly applied before the BukkitGradle plugin.
86-86: Added null safety with safe navigation operatorGood defensive programming practice by using the safe navigation operator (
?.) when accessing the task outcome, which prevents potential NullPointerExceptions if the task is not found.src/main/kotlin/server/DevServerPlugin.kt (3)
13-13: Updated import to use custom extension propertyChanged from importing JavaPluginExtension to a custom java extension property, which is part of the refactoring to reduce direct dependency on Java plugin extension classes.
57-58: Minor comment wording improvementSmall clarification in the comment wording.
68-68: Updated toolchain access to use extension propertyChanged from using
the<JavaPluginExtension>().toolchainto accessingjava.toolchaindirectly through the custom extension property, which aligns with the imports change above.src/main/kotlin/extensions/Provider.kt (1)
12-21: Clean implementation of mapNotNull extension functionThis is a well-implemented extension function that allows filtering and transforming Provider values in a null-safe way. The implementation correctly uses Java's Optional class to handle the nullable transformation and provides proper documentation about the unstable API usage.
The function will be useful for the resource factory integration where you need to transform configuration values that might be null.
src/main/kotlin/extensions/Accessors.kt (1)
13-17: Good use of Kotlin extensions for clean DSL accessThese extension properties provide a clean, idiomatic way to access the SourceSetContainer and ResourceFactoryExtension. Using Kotlin property delegation via
the()follows Gradle's Kotlin DSL patterns and will make the integration with resource-factory more readable.src/main/kotlin/Bukkit.kt (3)
11-12: Updated meta property return type to BukkitPluginYamlThis change properly updates the deprecated
metaproperty to work with the new resource factory-based system. The return type is nowBukkitPluginYamlinstead of the removedPluginConfigurationinterface.
21-23: Updated deprecation notice and implementation for generateMetaThe generateMeta property has been properly updated to point users to the new generatePluginYaml property, with a consistent ReplaceWith suggestion. The implementation now correctly delegates to the new property.
25-26: Added clear generatePluginYaml propertyThis new property provides a clean way to control plugin.yml generation with the resource factory. The property name and type (Provider) matches Gradle's conventions for configurable features.
src/main/kotlin/BukkitGradlePlugin.kt (3)
34-37: Simplified BukkitExtension creationThe BukkitExtension creation has been simplified by removing the dependency on PluginConfigurationImpl. This is a good refactoring that reduces coupling between components.
39-40: Proper plugin YAML feature configurationThe new configurePluginYamlFeature call properly replaces the previous PluginConfigurationPlugin logic, integrating with the resource factory system. The DevServerPlugin application remains unchanged.
42-46: Improved Java toolchain configurationThis change improves how the Java toolchain is configured by using the plugins.withType pattern, which is more idiomatic in Gradle's Kotlin DSL. The use of the java extension property from your custom accessors is a nice touch that makes the code more readable.
src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt (1)
16-20: Keep this mirror class in sync automaticallyRelying on a doc‑comment reminder to keep
BukkitPluginYamlDefaultsaligned withBukkitPluginYaml.Serializableis fragile.
A simple unit test that serialises/deserialises a sampleplugin.ymlwith both models (or at least checks field counts/names via reflection) will alert maintainers when upstream adds/removes properties.[ suggest_optional_refactor ]
src/main/kotlin/BukkitExtension.kt (1)
42-45:disableMetaGeneration()may fail after property is finalisedBecause
generatePluginYamlis finalised on first read, callingdisableMetaGeneration()after any task/plugin accessed the flag will throw.
Either:
- Mark the method as
@Incubatingand document the limitation, or- Turn the property into a
valbacked by a simplevar(nofinalizedOnRead) and mark it@Inputon tasks that need it.src/main/kotlin/plugin/PluginYamlFeature.kt (1)
54-60: 🛠️ Refactor suggestionPrematurely finalising
generatePluginYamlReading
bukkit.generatePluginYaml.get()during configuration finalises the property, preventing users from changing it later in the build script.
Useprovider { bukkit.generatePluginYaml.get() }insideonlyIf {}to defer evaluation until task execution.-onlyIf("Flag generatePluginYaml is enabled") { generatePluginYaml } +onlyIf("Flag generatePluginYaml is enabled") { bukkit.generatePluginYaml.get() }Likely an incorrect or invalid review comment.
src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
src/main/kotlin/BukkitGradlePlugin.kt (1)
31-34: Document breaking change regarding Java plugin applicationThe conditional application of the Java plugin is a breaking change that requires users to explicitly apply the Java plugin in their build scripts if they don't have another JVM language plugin.
This breaking change should be documented in README.md and CHANGELOG.md to inform users they need to explicitly apply the Java plugin in their build scripts.
#!/bin/bash # Check if README or CHANGELOG mention Java plugin application change grep -n "java plugin" README.md || echo "README doesn't mention Java plugin change" grep -n "java plugin" CHANGELOG.md || echo "CHANGELOG doesn't mention Java plugin change"src/main/kotlin/plugin/PluginYamlFeature.kt (2)
24-25: Hard‑coded"main"source‑set still limits advanced layouts
The TODO is kept, but the constant remains. Multi‑source‑set projects (e.g.shadow,api) cannot opt‑in.Exposing a Gradle property (defaulting to
"main") would make the feature reusable while preserving current behaviour.
88-91: Defaultmainclass may become an invalid identifier whengroupis emptyIf users don’t set
project.group, the generated value is".PascalCaseName"(note the leading dot), which is not a valid JVM class name and produces an unusableplugin.yml.Guard against an empty
groupor emit a clear error:-main.convention(name.mapNotNull { - if (project.group.toString().isEmpty()) return@mapNotNull null - "${project.group}.${StringUtils.toPascalCase(it)}" } -) +main.convention(name.map { + val pkg = project.group.toString() + require(pkg.isNotBlank()) { + "project.group must be set (e.g. 'com.example') to derive a default 'main' class" + } + "$pkg.${StringUtils.toPascalCase(it)}" +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
src/main/kotlin/BukkitExtension.kt(2 hunks)src/main/kotlin/BukkitGradlePlugin.kt(2 hunks)src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt(1 hunks)src/main/kotlin/plugin/PluginYamlFeature.kt(1 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/PluginSpecification.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt (1)
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt:22-43
Timestamp: 2025-04-17T15:02:54.544Z
Learning: In the BukkitGradle project, osipxd prefers to keep all fields nullable in data classes that mirror other structures (like BukkitPluginYamlDefaults) for consistency, even for collection types. This allows for uniform handling of all properties regardless of their type.
src/main/kotlin/plugin/PluginYamlFeature.kt (2)
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/PluginYamlFeature.kt:31-39
Timestamp: 2025-04-17T14:28:27.204Z
Learning: For charleskorn/kaml (Yaml implementation for kotlinx.serialization), SerialDescriptor components are thread-safe, but format objects themselves aren't explicitly documented as thread-safe. Best practice is to create separate instances per thread or use synchronization when sharing.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/PluginYamlFeature.kt:27-30
Timestamp: 2025-04-17T15:00:22.684Z
Learning: The SOURCE_SET_NAME constant in PluginYamlFeature.kt is intentionally kept as a hard-coded "main" value for simplicity, with plans to make it configurable later if demand arises.
src/main/kotlin/plugin/task/ParsePluginYaml.kt (3)
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:36-39
Timestamp: 2025-04-17T13:42:09.782Z
Learning: In the BukkitGradle plugin's task system, properties like `pluginYaml` in the `ParsePluginYaml` task are expected to be configured during the task configuration phase. Failing fast by using `property.get()` without null checks is intentional as missing required properties should fail early rather than with defensive programming.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:26-28
Timestamp: 2025-04-17T13:21:34.749Z
Learning: In Gradle, the `@SkipWhenEmpty` annotation on an `@InputFile` property will skip the task with `NO_SOURCE` outcome both when the file doesn't exist and when the file exists but is empty.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:26-28
Timestamp: 2025-04-17T13:21:34.749Z
Learning: In Gradle, the `@SkipWhenEmpty` annotation on an `@InputFile` property will cause the task to be skipped with a `NO_SOURCE` outcome both when the file doesn't exist and when the file exists but is empty. Gradle checks for the existence of inputs marked with `@SkipWhenEmpty` before executing the task action.
🧬 Code Graph Analysis (2)
src/main/kotlin/BukkitGradlePlugin.kt (2)
src/main/kotlin/plugin/PluginYamlFeature.kt (1)
configurePluginYamlFeature(27-65)src/main/kotlin/server/DevServerPlugin.kt (1)
apply(33-64)
src/main/kotlin/BukkitExtension.kt (1)
src/main/kotlin/Bukkit.kt (1)
ReplaceWith(8-27)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check
🔇 Additional comments (8)
src/test/groovy/ru/endlesscode/bukkitgradle/PluginSpecification.groovy (1)
85-86: Good defensive programming with safe navigation operatorUsing the safe navigation operator
?.prevents potentialNullPointerExceptionif the task is not found in the build result. This is particularly important with the removal of legacy tasks likemergePluginYaml.src/main/kotlin/BukkitGradlePlugin.kt (3)
36-39: Updated BukkitExtension creation aligns with new plugin YAML architectureThe
BukkitExtensionis now created with only the server configuration, removing the previously used plugin configuration. This change supports the new plugin YAML feature architecture.
41-42: Good architecture: Plugin YAML feature configuration and server plugin applicationSeparating the plugin YAML configuration into a dedicated feature module and applying the server plugin directly improves code organization and maintainability.
44-46: Improved Java toolchain configurationThe Java toolchain configuration is now more cleanly implemented using the
java.toolchainextension with a proper convention setting based on the Bukkit API version.src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt (2)
16-43: Good design: Nullable defaults data class for plugin YAMLCreating a dedicated data class with nullable fields for plugin YAML defaults provides flexibility and allows for consistent handling of all properties. The comment explaining the design decision is helpful.
Based on the retrieved learning, I understand your preference for keeping all fields nullable for consistency, even for collection types. This approach allows for uniform property handling, which is sensible given the class mirrors another structure.
62-79: Well-implemented custom serializer with detailed error messagesThe
PermissionDefaultSerializerprovides good error handling with detailed messages that include allowed values when an unknown permission default is encountered.The implementation properly handles case-insensitive matching and provides clear error messages, addressing the concern raised in a previous review.
src/main/kotlin/plugin/task/ParsePluginYaml.kt (2)
51-54: 👍 Good call on adding debug logging for empty YAML
Logging the swallowedEmptyYamlDocumentExceptionatdebugpreserves valuable context while keeping normal output clean.
79-85: Per‑entry merging of commands & permissions looks great
UsingmaybeCreate(name)followed byapplyConventionsachieves the “merge don’t replace” behaviour discussed earlier—nice improvement!
src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy
Outdated
Show resolved
Hide resolved
src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy
Outdated
Show resolved
Hide resolved
src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md(1 hunks)src/main/kotlin/extensions/Provider.kt(1 hunks)src/main/kotlin/plugin/PluginYamlFeature.kt(1 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy(0 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy(1 hunks)
💤 Files with no reviewable changes (1)
- src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy
🧰 Additional context used
🧠 Learnings (2)
src/main/kotlin/plugin/PluginYamlFeature.kt (2)
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/PluginYamlFeature.kt:31-39
Timestamp: 2025-04-17T14:28:27.204Z
Learning: For charleskorn/kaml (Yaml implementation for kotlinx.serialization), SerialDescriptor components are thread-safe, but format objects themselves aren't explicitly documented as thread-safe. Best practice is to create separate instances per thread or use synchronization when sharing.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/PluginYamlFeature.kt:27-30
Timestamp: 2025-04-17T15:00:22.684Z
Learning: The SOURCE_SET_NAME constant in PluginYamlFeature.kt is intentionally kept as a hard-coded "main" value for simplicity, with plans to make it configurable later if demand arises.
src/main/kotlin/plugin/task/ParsePluginYaml.kt (4)
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:36-39
Timestamp: 2025-04-17T13:42:09.782Z
Learning: In the BukkitGradle plugin's task system, properties like `pluginYaml` in the `ParsePluginYaml` task are expected to be configured during the task configuration phase. Failing fast by using `property.get()` without null checks is intentional as missing required properties should fail early rather than with defensive programming.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:26-28
Timestamp: 2025-04-17T13:21:34.749Z
Learning: In Gradle, the `@SkipWhenEmpty` annotation on an `@InputFile` property will skip the task with `NO_SOURCE` outcome both when the file doesn't exist and when the file exists but is empty.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:26-28
Timestamp: 2025-04-17T13:21:34.749Z
Learning: In Gradle, the `@SkipWhenEmpty` annotation on an `@InputFile` property will cause the task to be skipped with a `NO_SOURCE` outcome both when the file doesn't exist and when the file exists but is empty. Gradle checks for the existence of inputs marked with `@SkipWhenEmpty` before executing the task action.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: src/main/kotlin/plugin/task/ParsePluginYaml.kt:3-3
Timestamp: 2025-04-17T15:28:50.784Z
Learning: Wildcard imports (e.g., `import package.*`) are allowed and acceptable in the BukkitGradle project, contrary to the common practice of using explicit imports.
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
21-21: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
24-24: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
🔇 Additional comments (2)
src/main/kotlin/plugin/PluginYamlFeature.kt (1)
58-63: Resource directory deduplication can drop custom directories
resources.setSrcDirs(resources.srcDirs.distinct())keeps order but silently removes duplicates only from the main source‑set.
If another plugin has already added additional resource directories, they may be lost when duplicates are removed.Consider applying
distinct()only to the list returned byresources.srcDirsand passing the merged set back, preserving previously added dirs:val unique = resources.srcDirs.toMutableSet() // preserves extras resources.setSrcDirs(unique)src/main/kotlin/plugin/task/ParsePluginYaml.kt (1)
77-83: Null‑safety guard forcommandsandpermissionscollectionsIf
commandsorpermissionsare nullable inBukkitPluginYamlDefaults(e.g. decoded asMap<String, Command>? = null), iterating without a null‑check will throw.
Using safe iteration prevents an intermittent NPE at runtime:-for ((name, command) in defaults.commands) { +for ((name, command) in defaults.commands.orEmpty()) { commands.maybeCreate(name).applyConventions(command) } -for ((name, permission) in defaults.permissions) { +for ((name, permission) in defaults.permissions.orEmpty()) { permissions.maybeCreate(name).applyConventions(permission) }
src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy
Show resolved
Hide resolved
src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy
Show resolved
Hide resolved
osipxd
force-pushed
the
use-plugin-yml-plugin
branch
from
b9b0693 to
79f748d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (4)
75-78: Add missing determiners in Quick Start.
- “Apply the plugin and configure the project’s
group,description, andversion.”- “These values will be used to generate the plugin.yml file:”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: You might be missing the article “the” here.
Context: ...k Start Apply the plugin and configure project'sgroup,descriptionandversion....(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~78-~78: You might be missing the article “the” here.
Context: .... These values will be used to generateplugin.ymlfile: ```kotlin plugins { ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
86-86: Plugin description assignment is valid.
Consider adding a trailing period for consistency in example code.
96-96: Inline comment capitalization.
Consider starting the comment with “See” (capital “S”) for consistency with sentence style.
104-107: Verify exampleplugin.ymlsnippet against actual output.
Since generation now usesresource‑factory, ensure keys/format match the real file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)README.md(8 hunks)src/main/kotlin/BukkitExtension.kt(2 hunks)src/main/kotlin/extensions/Provider.kt(1 hunks)src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt(1 hunks)src/main/kotlin/plugin/PluginYamlFeature.kt(1 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy(0 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy(1 hunks)
💤 Files with no reviewable changes (1)
- src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/kotlin/plugin/BukkitPluginYamlDefaults.kt
- src/main/kotlin/extensions/Provider.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/GeneratePluginYamlSpec.groovy
- src/main/kotlin/plugin/PluginYamlFeature.kt
- src/main/kotlin/plugin/task/ParsePluginYaml.kt
- src/main/kotlin/BukkitExtension.kt
- CHANGELOG.md
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~77-~77: You might be missing the article “the” here.
Context: ...k Start Apply the plugin and configure project's group, description and version....
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~78-~78: You might be missing the article “the” here.
Context: .... These values will be used to generate plugin.yml file: ```kotlin plugins { ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check
🔇 Additional comments (23)
README.md (23)
37-38: Installation requirement note is clear.
Specifies that Gradle 8.0+ is required, which aligns with the plugin’s minimum supported Gradle version.
46-46: Plugins portal link is correct.
The URL points to the right Gradle Plugin Portal entry forru.endlesscode.bukkitgradle.
48-48: Use of<details>improves readability.
Collapsing snapshot instructions helps keep the main README focused.
50-50: Snapshot summary label is appropriate.
The<summary>text clearly indicates the snapshot usage section.
52-52: Snapshot instructions are concise and actionable.
Explains how to configurejitpackand apply thedevelop-SNAPSHOTversion.
73-74: Details block is closed correctly.
The end tag and spacing maintain documentation structure.
89-91: Updatedbukkit { apiVersion }example is correct.
Demonstrates the new extension property for API versioning.
93-93: Guidance comment for dependency section is helpful.
Directs users to the full dependencies reference.
100-102: Quick Start conclusion is clear.
Signals that plugin.yml generation happens automatically on build.
111-113: Default main-class pattern note is helpful.
Clearly explains how themainproperty is inferred if not overridden.
118-119: Configuration section header updated correctly.
Switch tobukkit.pluginblock is well documented.
122-124: API version default comment is accurate.
Indicates the fallback whenapiVersionis not explicitly set.
125-125: Starting plugin configuration comment is clear.
Separates general settings from detailed plugin.yml properties.
127-132: Plugin properties block is correct.
Using direct assignments (instead of setters) aligns with the new Kotlin DSL.
139-140: Repositories & Dependencies introduction updated.
Clearly informs users about the provided shortcuts.
143-144:spigot()shortcut example is valid.
Matches the new extension function in the API.
147-148:compileOnly(paperApi())example is accurate.
Demonstrates the recommended dependency usage.
260-270: Migration Guide step for deprecated API is clear.
Showsmeta {}→plugin {}andurl→websitemigrations accurately.
275-282: Upgrade from 0.8.x: API version rename is precise.
Changingversion→apiVersionis correctly documented.
283-292: Build server block migration is consistent.
Switching fromruntoserverblock aligns with refactored API.
293-302: JVM args syntax update is helpful.
Presents both the array form and the method call alternative.
303-312: Repository & dependency remappings are correct.
Mappingdestroystokyo()→papermc()andvault()→jitpack()is accurate.
318-319: Final migration steps are clearly stated.
Reminds users to remove legacy functions and verify generated YAML.
osipxd
force-pushed
the
use-plugin-yml-plugin
branch
from
79f748d to
1811498
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
111-112: Add missing article in note
To improve readability, include “the” before “main class”:- > By default, main class is built by the following pattern: `<groupId>.<name>` + > By default, the main class is built by the following pattern: `<groupId>.<name>`🧰 Tools
🪛 LanguageTool
[uncategorized] ~112-~112: You might be missing the article “the” here.
Context: ... by Gradle ``` > [!NOTE] > By default, main class is built by the following pattern...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(8 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~101-~101: A comma might be missing here.
Context: ...nfo } ``` That's it! During the plugin compilation plugin.yml will be generated with the...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~112-~112: You might be missing the article “the” here.
Context: ... by Gradle ``` > [!NOTE] > By default, main class is built by the following pattern...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
README.md
75-75: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
116-116: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
137-137: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
151-151: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
151-151: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
165-165: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
253-253: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
274-274: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check
🔇 Additional comments (4)
README.md (4)
37-38: Clarify Gradle version requirement
The new note explicitly states that Gradle 8.0+ is required, which is essential for users to know before attempting to apply the plugin. This improves clarity.
46-46: Link to plugin portal added
Including the direct link to the plugin on plugins.gradle.org makes it easier for users to find and install the plugin. Looks good.
73-74: Closing<details>block
The</details>tag correctly closes the collapsible snapshot instructions. No action needed here.
84-97:⚠️ Potential issueAlign plugin version in the Quick Start example
The example shows version0.10.1, but with the new resource-factory integration the next release will likely be0.11.0. Please update the snippet to reflect the correct version:- plugins { - id("ru.endlesscode.bukkitgradle") version "0.10.1" - } + plugins { + id("ru.endlesscode.bukkitgradle") version "0.11.0" + }Likely an incorrect or invalid review comment.
osipxd
force-pushed
the
use-plugin-yml-plugin
branch
from
1811498 to
22050c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
README.md (8)
37-38: Clarify wording of Gradle requirement
Consider tweaking the note for smoother readability, for example:- > BukkitGradle requires Gradle 8.0+ to run + > BukkitGradle requires Gradle 8.0 or newer to run
75-75: Consistent heading style
This level‑3 heading is atx‑style (###), but the project currently prefers setext‑style for third‑level sections. Consider switching to setext for consistency:Quick Start ------------🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
75-75: Heading style
Expected: setext; Actual: atx(MD003, heading-style)
77-77: Add Oxford comma in instructional sentence
For clarity, insert a comma before “and” in the list:- Apply the plugin and configure project's `group`, `description` and `version`. + Apply the plugin and configure your project's `group`, `description`, and `version`.
96-96: Link to Dependencies section
Use backticks or a Markdown link to reference the section for consistency, e.g.:- // See the 'Dependencies' section for more info + // See the [`Dependencies`](#repositories-and-dependencies) section for more info
104-108: Unify quoting in YAML example
descriptioncontains spaces and should be quoted consistently. Consider quoting all string values for clarity:api-version: '1.16' name: "MyPlugin" -version: '0.1' +version: '0.1' main: "com.example.myplugin.MyPlugin" -description: My first Bukkit plugin built by Gradle +description: "My first Bukkit plugin built by Gradle"
111-112: Include missing article “the”
Add “the” for grammatical correctness:- > By default, main class is built by the following pattern… + > By default, the main class is built by the following pattern…🧰 Tools
🪛 LanguageTool
[uncategorized] ~112-~112: You might be missing the article “the” here.
Context: ... by Gradle ``` > [!NOTE] > By default, main class is built by the following pattern...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
122-122: Tighten default‐value comment
For consistency and grammar, consider:- // Version of API. By default, 1.16.5 is used + // API version to target; defaults to 1.16.5
178-178: Hyphenation in “extension‑functions”
Consider using “extension functions” without a hyphen for readability:- If you need more extension-functions, [file an issue][issue]. + If you need more extension functions, [file an issue][issue].
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (2)
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: README.md:104-108
Timestamp: 2025-04-17T20:35:24.623Z
Learning: In Bukkit plugin.yml, the `api-version` field doesn't support patch versions for Minecraft versions lower than 1.20.5. Even if `apiVersion = "1.16.5"` is specified in configuration, it will be transformed to `api-version: '1.16'` in the generated plugin.yml file. Starting from Minecraft 1.20.5, full patch versions are supported in the api-version field.
Learnt from: osipxd
PR: EndlessCodeGroup/BukkitGradle#66
File: README.md:104-108
Timestamp: 2025-04-17T20:35:24.623Z
Learning: In Bukkit plugin.yml, the `api-version` field doesn't support patch versions for Minecraft versions lower than 1.20.5. Even if `apiVersion = "1.16.5"` is specified in configuration, it will be transformed to `api-version: '1.16'` in the generated plugin.yml file. Starting from Minecraft 1.20.5, full patch versions are supported in the api-version field.
🪛 LanguageTool
README.md
[uncategorized] ~101-~101: A comma might be missing here.
Context: ...nfo } ``` That's it! During the plugin compilation plugin.yml will be generated with the...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~112-~112: You might be missing the article “the” here.
Context: ... by Gradle ``` > [!NOTE] > By default, main class is built by the following pattern...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~317-~317: The preposition ‘to’ seems more likely in this position.
Context: ... 5. Remove q and qq functions calls in meta { ... } 6. Check generated plugi...
(AI_HYDRA_LEO_REPLACE_IN_TO)
🪛 markdownlint-cli2 (0.17.2)
README.md
75-75: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
116-116: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
137-137: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
151-151: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
151-151: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
165-165: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
253-253: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
274-274: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check
🔇 Additional comments (10)
README.md (10)
12-12: TOC entry matches renamed section
The Table of Contents has been updated to reference “Quick Start” instead of “First steps,” and the anchor#quick-startcorrectly aligns with the heading.
46-46: Plugin link added for convenience
Linking to the Gradle Plugin Portal here is helpful for users discovering the plugin.
86-86: Example description assignment looks good
The sampledescription = "My first Bukkit plugin built by Gradle"aligns with the Quick Start narrative.
90-91: Show completebukkit { … }block
Including both opening and closing braces makes the snippet self‑contained and clear.
93-93: Descriptive comment for adding API
The inline comment// Add the necessary API to the projectclearly explains the next step.
118-118: Clarify plugin configuration block
The description of thebukkit.plugin { … }block is concise and accurate.
127-132: Plugin metadata example is clear
The sample assignments forname,description,main,version,authors, anddependalign with the new API and are well‑structured.
253-253: Migration section heading updated
The guide now references upgrading from 0.10.x, which matches the plugin version in the Quick Start.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
253-253: Heading style
Expected: setext; Actual: atx(MD003, heading-style)
255-255: Gradle update instruction is clear
Advising Gradle 8.0+ and showing the wrapper command provides a smooth upgrade path.
264-268: Migration diff reflects API renames correctly
The diff shows replacingmetawithpluginandurlwithwebsite, aligning with the newbukkit.pluginDSL.
TODO:
Closes #11