Rename meta to plugin #64
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes update the BukkitGradle plugin by renaming all instances of “meta” to “plugin”. This includes configuration blocks in the README and CHANGELOG, public API elements (properties, methods, tasks), and package imports. Deprecated elements are maintained for backward compatibility while new implementations are introduced. Several legacy files related to metadata handling have been removed, and new files managing YAML configuration are added. Tests have been updated accordingly to reflect these naming and functional shifts. Changes
Sequence Diagram(s)sequenceDiagram
participant P as Project
participant PCP as PluginConfigurationPlugin
participant PT as ParsePluginYamlTask
participant MT as MergePluginYamlTask
participant PR as ProcessResources Task
P->>+PCP: apply()
PCP-->>P: Registers plugin configuration
PCP->>PT: Register and configure parsePluginYaml task
PCP->>MT: Register and configure mergePluginYaml task
PT->>MT: Provide parsed YAML data
MT->>PR: Output merged plugin.yml to resources
PR-->>P: Include plugin.yml in build
Assessment against linked issues
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (21)
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (3)
src/main/kotlin/plugin/task/ParsePluginYaml.kt (1)
1-69: Well-structured implementation for plugin YAML parsingThis new class provides a robust implementation for parsing plugin.yml files, with proper handling of cases where the file might be missing or empty. The code follows good Kotlin and Gradle task conventions.
Consider expanding the exception handling to catch more potential YAML parsing exceptions beyond just
EmptyYamlDocumentException. For example, adding a generic exception catch block as a fallback could make the code more resilient against malformed YAML files.try { file.inputStream().use { yaml.decodeFromStream<PluginYaml>(it) } } catch (cause: EmptyYamlDocumentException) { return PluginYaml() +} catch (cause: Exception) { + logger.warn("Failed to parse plugin.yml file: ${cause.message}") + return PluginYaml() }src/main/kotlin/BukkitGradlePlugin.kt (1)
48-57: Return type of configurePluginMeta updated to PluginConfigurationImpl.The function implementation remains functionally equivalent while using the new implementation class. The function name
configurePluginMetacould be renamed toconfigurePluginConfigurationin a future update for consistency.Consider renaming the
configurePluginMeta()method toconfigurePluginConfiguration()in a future update to maintain consistent naming throughout the codebase.src/main/kotlin/BukkitExtension.kt (1)
62-66: Deprecation annotation for meta-generation is consistent with the rename.
Documenting the shift toplugin.disablePluginYamlGeneration()and marking this method as deprecated is a solid approach for easing transitions.Consider adding a future removal version or referencing the TODO on line 17 in the deprecation message, so users clearly understand when it will be removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG.md(1 hunks)README.md(1 hunks)src/main/kotlin/Bukkit.kt(2 hunks)src/main/kotlin/BukkitExtension.kt(3 hunks)src/main/kotlin/BukkitGradlePlugin.kt(3 hunks)src/main/kotlin/dependencies/Dependencies.kt(1 hunks)src/main/kotlin/extensions/Accessors.kt(1 hunks)src/main/kotlin/meta/PluginMetaPlugin.kt(0 hunks)src/main/kotlin/meta/task/MergePluginMeta.kt(0 hunks)src/main/kotlin/meta/task/ParsePluginMetaFile.kt(0 hunks)src/main/kotlin/plugin/PluginConfigurationPlugin.kt(1 hunks)src/main/kotlin/plugin/PluginYaml.kt(3 hunks)src/main/kotlin/plugin/extension/PluginConfiguration.kt(1 hunks)src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt(2 hunks)src/main/kotlin/plugin/task/MergePluginYaml.kt(1 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/main/kotlin/plugin/util/MinecraftVersion.kt(1 hunks)src/main/kotlin/plugin/util/StringUtils.kt(1 hunks)src/main/kotlin/server/DevServerPlugin.kt(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy(13 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/util/StringUtilsSpec.groovy(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/kotlin/meta/task/MergePluginMeta.kt
- src/main/kotlin/meta/task/ParsePluginMetaFile.kt
- src/main/kotlin/meta/PluginMetaPlugin.kt
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/kotlin/BukkitGradlePlugin.kt (1)
src/main/kotlin/plugin/PluginConfigurationPlugin.kt (2)
apply(20-69)apply(22-56)
src/main/kotlin/BukkitExtension.kt (2)
src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt (6)
ReplaceWith(30-34)ReplaceWith(36-40)ReplaceWith(42-46)ReplaceWith(48-52)ReplaceWith(54-58)ReplaceWith(60-64)src/main/kotlin/Bukkit.kt (1)
plugin(7-26)
src/main/kotlin/Bukkit.kt (2)
src/main/kotlin/BukkitExtension.kt (4)
ReplaceWith(37-41)ReplaceWith(51-54)ReplaceWith(56-60)ReplaceWith(63-69)src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt (6)
ReplaceWith(30-34)ReplaceWith(36-40)ReplaceWith(42-46)ReplaceWith(48-52)ReplaceWith(54-58)ReplaceWith(60-64)
🔇 Additional comments (45)
src/main/kotlin/extensions/Accessors.kt (1)
1-8: Clean implementation of a convenience extension propertyThis extension property provides a concise way to access the Java plugin extension, which is a good practice when working with Gradle Kotlin DSL. The implementation is clean and properly scoped as internal.
src/main/kotlin/plugin/util/StringUtils.kt (1)
1-1: Package name update aligns with PR objectivesThe package name change from
meta.utiltoplugin.utilis consistent with the PR's goal of renaming "meta" to "plugin" throughout the codebase.src/main/kotlin/plugin/util/MinecraftVersion.kt (1)
1-1: Package name update aligns with PR objectivesThe package name change from
meta.utiltoplugin.utilis consistent with the PR's goal of renaming "meta" to "plugin" throughout the codebase.src/test/groovy/ru/endlesscode/bukkitgradle/plugin/util/StringUtilsSpec.groovy (1)
1-1: Test package updated to match implementation packageThe package name change in the test file correctly aligns with the corresponding implementation class, ensuring tests continue to work with the renamed package structure.
src/main/kotlin/plugin/extension/PluginConfiguration.kt (1)
1-11: Interface rename and new configuration propertyThe package and interface renaming from
meta.extension.PluginMetatoplugin.extension.PluginConfigurationbetter reflects its purpose. The addition of thegeneratePluginYamlproperty with the@Internalannotation provides a new configurable option for controlling YAML generation without affecting task up-to-date checking.README.md (1)
126-126: Approve configuration rename frommetatopluginThe configuration block for plugin metadata has been appropriately renamed from
metatoplugin. This change aligns with the overall renaming effort mentioned in the PR objectives.src/main/kotlin/server/DevServerPlugin.kt (1)
14-14: Import path updated correctlyThe import path has been appropriately updated from
ru.endlesscode.bukkitgradle.meta.util.resolveMinimalJavaVersiontoru.endlesscode.bukkitgradle.plugin.util.resolveMinimalJavaVersion, which aligns with the package renaming from "meta" to "plugin".src/main/kotlin/dependencies/Dependencies.kt (1)
15-16: Import paths updated correctlyThe import paths for
MinecraftVersionandparsedApiVersionhave been properly updated to use the new package structureru.endlesscode.bukkitgradle.plugin.utilinstead of the previousru.endlesscode.bukkitgradle.meta.util. This change is consistent with the overall package renaming effort.CHANGELOG.md (1)
12-16: Documentation for breaking changes is clear and comprehensive.The changelog entry properly documents the breaking changes, specifically outlining:
- The renaming of configuration block from
bukkit.metatobukkit.plugin- The task renaming changes
- API method replacements
- Package renaming
This provides users with clear guidance on what changes they need to make to migrate to the new version.
src/main/kotlin/plugin/PluginYaml.kt (4)
1-7: Naming change from PluginMetaYaml to PluginYaml is appropriate.This renaming is consistent with the overall goal of changing "meta" to "plugin" throughout the codebase. The package name has also been updated accordingly.
40-41: Addition of contributors field enhances plugin metadata capabilities.The new
contributorsfield allows plugin authors to properly credit contributors separately from the main authors, improving metadata completeness.
55-61: New 'provides' field adds important functionality for plugin dependency resolution.This addition allows plugins to declare alternative implementations they provide, which helps with dependency resolution when other plugins depend on specific functionality rather than specific implementations.
72-74: Default permission field follows Bukkit plugin.yml convention.The
defaultPermissionproperty with appropriate@SerialName("default-permission")annotation ensures compatibility with Bukkit's plugin.yml specification for defining default permission values.src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt (5)
1-14: Class renamed from PluginMetaImpl to PluginConfigurationImpl to reflect new naming convention.The implementation has been updated to implement
PluginConfigurationinstead of the previousPluginMetainterface, maintaining consistent renaming throughout the codebase.
15-19: Property for controlling plugin.yaml generation follows Gradle best practices.The implementation uses a private property with finalization on read, which follows Gradle's best practices for property configuration. The public property provides type-safe access to the underlying value.
28-28: Logger name updated to match the new class name.Consistency is maintained by updating the logger name from "PluginMeta" to "PluginConfiguration".
31-64: Deprecated methods have updated warning messages to reflect new naming.The warning messages in deprecated setter methods have been updated to use the new
bukkit.pluginnaming instead ofbukkit.meta, ensuring users receive appropriate migration guidance when using these methods.
66-69: New method for disabling plugin.yaml generation added.This method is the replacement for the previous
disableMetaGeneration()method mentioned in the changelog. It provides a clean way to disable YAML generation when needed.src/main/kotlin/BukkitGradlePlugin.kt (2)
9-14: Imports updated to reflect package renaming from 'meta' to 'plugin'.All import statements have been consistently updated to use the new package structure, maintaining the overall renaming pattern.
37-37: PluginMetaPlugin replaced with PluginConfigurationPlugin.The plugin application has been updated to use the new plugin class, consistent with the renaming effort.
src/main/kotlin/Bukkit.kt (4)
4-4: Updated import to reflect the new plugin-oriented namingThe import statement has been updated from a meta-related import to the new plugin-oriented naming structure, which matches the renaming goal of the PR.
9-11: Property renamed from 'meta' to 'plugin'The property has been renamed from 'meta' to 'plugin', which aligns with the PR objective to rename all instances of "meta" to "plugin" throughout the codebase.
12-14: Proper deprecation of the 'meta' propertyThe 'meta' property is correctly marked as deprecated with a helpful message directing users to the new 'plugin' property. The implementation returns the new 'plugin' property to maintain backward compatibility.
23-25: Proper deprecation of 'generateMeta' propertyThe 'generateMeta' property is correctly marked as deprecated with a helpful message directing users to the new 'plugin.generatePluginYaml' property. The implementation returns the new property to maintain backward compatibility.
src/main/kotlin/plugin/PluginConfigurationPlugin.kt (4)
1-19: Well-organized importsThe plugin imports are well-organized and include all necessary dependencies for the plugin's functionality, including YAML processing, Gradle API, and project-specific imports.
20-56: Well-structured plugin implementationThe
PluginConfigurationPluginclass is well-implemented with:
- Proper YAML configuration setup with flow sequence style
- Registration of two tasks (
parsePluginYamlandmergePluginYaml) with appropriate configurations- Integration with the
processResourcestask- Proper handling of conditional execution based on the
generatePluginYamlpredicateThe implementation follows Gradle's best practices for plugin development.
58-64: Helpful utility method for finding plugin.yamlThe
findPluginYamlmethod is well-documented and provides a clear utility for locating the plugin's YAML file in the project's resource directory. The use oftakeIfis a nice touch for returning null if the file doesn't exist.
66-68: Companion object with constantThe companion object contains a constant for the plugin file name, which promotes consistency and reduces magic strings throughout the codebase.
src/main/kotlin/plugin/task/MergePluginYaml.kt (5)
1-17: Complete and organized importsThe imports are well-organized and include all necessary dependencies for the task's functionality, including YAML processing, Gradle API, and task-specific annotations.
18-26: Well-documented task class with proper annotationsThe
MergePluginYamlclass is:
- Well-documented with Javadoc including a reference to the related plugin class
- Properly annotated with
@Injectfor dependency injection- Extends
DefaultTaskas required for a Gradle task- Has appropriate constructor parameters
These elements follow Gradle's best practices for task implementation.
28-38: Properly annotated propertiesThe properties are well-annotated with appropriate Gradle task input/output annotations:
@Internalfor the YAML instance that's not part of the task's inputs@Nestedfor the plugin configuration@Inputfor the pluginYaml property that serves as input@OutputFilefor the target file propertyThese annotations ensure Gradle's incremental build functionality works correctly with this task.
40-46: Well-structured initialization blockThe initialization block:
- Sets the appropriate Gradle task group and description
- Provides a sensible default for the target file location
- Uses the task's temporary directory for the default location
This follows Gradle's task configuration best practices.
48-64: Clear and efficient implementation of the task actionThe
mergePluginYamlmethod:
- Is properly annotated with
@TaskAction- Creates a merged configuration by copying the input and updating specific fields
- Only includes non-empty values for optional fields like description, apiVersion, website, and authors
- Efficiently writes the serialized YAML to the target file
The implementation is clean and follows good practices for task actions.
src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy (7)
6-6: Updated import to reference the new plugin packageThe import has been updated to reference the new
PluginConfigurationPluginclass instead of a metadata-related class.
9-9: Renamed test class to reflect new terminologyThe class has been renamed from
MergePluginMetaSpectoMergePluginYamlSpecto match the renaming from "meta" to "plugin" throughout the codebase.
11-14: Updated constant and file referencesThe task path constant and file references have been updated to use the new naming convention, changing from meta-related terms to plugin-related terms.
17-18: Updated file paths in setup methodThe file paths in the setup method now reference the plugin YAML file using the constant from
PluginConfigurationPlugin.FILE_NAMEinstead of hardcoded meta-related file names.
25-34: Updated test case description and assertionsThe test case description and assertions have been updated to reference the new task names (
mergePluginYamlandparsePluginYaml) instead of the old meta-related task names.
36-48: Updated test case for disabled generationThe test case for disabled generation still uses the deprecated
disableMetaGeneration()method even though we're checking for the new plugin-related tasks. This is likely intentional to test backward compatibility with the deprecated method.
50-61: Updated test cases with new file references and expectationsAll the test cases have been updated to:
- Reference the plugin YAML file instead of the meta file
- Use the new task names in descriptions and assertions
- Update the plugin configuration block to use
plugin { ... }instead ofmeta { ... }- Maintain the same test coverage but with the new terminology
These changes maintain comprehensive test coverage while adapting to the new naming convention.
Also applies to: 63-87, 88-97, 99-120, 122-152, 154-183, 185-214, 216-242, 244-261, 263-290
src/main/kotlin/BukkitExtension.kt (5)
13-13: No concerns regarding the new import.
This import statement aligns with the renamed implementation class, ensuring a consistent reference to PluginConfigurationImpl.
19-19: Newpluginproperty override looks good.
Defining this override clarifies usage for downstream code, cleanly replacing older "meta" references.
47-49: Function-basedplugin { ... }configuration is well-structured.
This matches Gradle’s idiomatic style, allowing users to configure plugin properties through an action block.
51-53: Deprecation method provides smooth migration.
Maintains backward compatibility by delegating calls to the newplugin(body)method.
68-68: Delegation toplugin.disablePluginYamlGeneration()is correct.
Propagating the action to the new method is consistent with your renaming strategy and ensures a single code path for disabling plugin YAML generation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt (1)
12-12: Consider updating the TODO comment.The TODO comment refers to removing deprecated methods in version 1.0, but doesn't reflect the recent renaming changes. Consider updating it to be more specific about which deprecated methods should be removed.
-// TODO 1.0: Remove deprecated methods +// TODO 1.0: Remove deprecated setter methods (setName, setDescription, etc.)CHANGELOG.md (1)
12-16: Possible minor spelling error.We suspect "bukit" might be a typographical error for "bukkit." Please confirm or fix for consistency.
- `bukit.disableMetaGeneration()` replaced by `bukkit.plugin.disablePluginYamlGeneration()` + `bukkit.disableMetaGeneration()` replaced by `bukkit.plugin.disablePluginYamlGeneration()`src/main/kotlin/plugin/task/ParsePluginYaml.kt (2)
23-28: Consider avoidinglateinit varfor better safety.
@get:Internal lateinit varfields can increase the risk of uninitialized usage if not set as intended. As an optional improvement, consider initializing them asvalproperties or providing default values where possible.
43-46: Enhance error handling on file parsing.Currently, if
readFromFilethrows an unhandled IO or YAML parsing exception (other thanEmptyYamlDocumentException), the build may fail with a less descriptive error. You might consider wrapping additional exceptions to provide a clearer message or default behavior.src/main/kotlin/plugin/task/MergePluginYaml.kt (2)
28-32: Consider convertingyamlandpluginto abstract properties instead of usinglateinit.
Gradle tasks often encourage usingPropertyorObjectFactoryinjection for lazy configuration. Converting these fields to abstract properties (similar totargetandpluginYaml) can help ensure they’re initialized correctly and remain compatible with Gradle’s incremental builds.
50-64: Ensure the target directory is created before writing the YAML file.
If the parent directory oftarget.get().asFiledoes not exist, this operation may fail. Consider creating the directory structure first, for instance:public fun mergePluginYaml() { val merged = pluginYaml.get().copy( // ... ) + target.get().asFile.parentFile.mkdirs() target.get() .asFile .writeText(yaml.encodeToString(merged)) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG.md(1 hunks)README.md(1 hunks)src/main/kotlin/Bukkit.kt(2 hunks)src/main/kotlin/BukkitExtension.kt(3 hunks)src/main/kotlin/BukkitGradlePlugin.kt(3 hunks)src/main/kotlin/dependencies/Dependencies.kt(1 hunks)src/main/kotlin/extensions/Accessors.kt(1 hunks)src/main/kotlin/meta/PluginMetaPlugin.kt(0 hunks)src/main/kotlin/meta/task/MergePluginMeta.kt(0 hunks)src/main/kotlin/meta/task/ParsePluginMetaFile.kt(0 hunks)src/main/kotlin/plugin/PluginConfigurationPlugin.kt(1 hunks)src/main/kotlin/plugin/PluginYaml.kt(3 hunks)src/main/kotlin/plugin/extension/PluginConfiguration.kt(1 hunks)src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt(2 hunks)src/main/kotlin/plugin/task/MergePluginYaml.kt(1 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/main/kotlin/plugin/util/MinecraftVersion.kt(1 hunks)src/main/kotlin/plugin/util/StringUtils.kt(1 hunks)src/main/kotlin/server/DevServerPlugin.kt(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy(13 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/util/StringUtilsSpec.groovy(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/kotlin/meta/task/MergePluginMeta.kt
- src/main/kotlin/meta/task/ParsePluginMetaFile.kt
- src/main/kotlin/meta/PluginMetaPlugin.kt
🚧 Files skipped from review as they are similar to previous changes (12)
- src/main/kotlin/plugin/util/StringUtils.kt
- src/main/kotlin/plugin/util/MinecraftVersion.kt
- src/main/kotlin/extensions/Accessors.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/plugin/util/StringUtilsSpec.groovy
- src/main/kotlin/plugin/extension/PluginConfiguration.kt
- README.md
- src/main/kotlin/dependencies/Dependencies.kt
- src/main/kotlin/server/DevServerPlugin.kt
- src/main/kotlin/plugin/PluginYaml.kt
- src/main/kotlin/Bukkit.kt
- src/main/kotlin/plugin/PluginConfigurationPlugin.kt
- src/main/kotlin/BukkitExtension.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/kotlin/BukkitGradlePlugin.kt (1)
src/main/kotlin/plugin/PluginConfigurationPlugin.kt (2)
apply(20-69)apply(22-56)
🔇 Additional comments (10)
src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt (5)
1-1: Class declaration properly renamed to follow the new naming convention.The class has been successfully renamed from
PluginMetaImpltoPluginConfigurationImplwith correct implementation of thePluginConfigurationinterface, maintaining consistency with the PR objective to rename "meta" to "plugin" throughout the codebase.Also applies to: 9-9, 13-14
15-18: New property implementation looks good.The addition of the private
_generatePluginYamlproperty with appropriate default value (true) and finalizing on read is well-implemented. The public override provides proper interface compatibility while encapsulating the implementation details.
28-28: Logger name updated properly.The logger name has been correctly updated from "PluginMeta" to "PluginConfiguration" to maintain consistency with the class name change.
32-32: Deprecation warning messages updated consistently.All deprecation warning messages have been consistently updated to use the new "bukkit.plugin" prefix instead of "bukkit.meta", which aligns with the renaming objective of the PR.
Also applies to: 38-38, 44-44, 50-50, 56-56, 62-62
66-69: New methoddisablePluginYamlGenerationfunctionality looks good.The new method provides a clear and concise way to disable plugin.yml generation with appropriate documentation. This replaces previous functionality while improving the API design.
src/main/kotlin/plugin/task/ParsePluginYaml.kt (1)
49-68: Logic for reading and populatingPluginYamllooks good.Strong use of
try/catchfor empty YAML documents, plus default initialization ofPluginYamlif missing or empty. This approach is concise, and applying Gradle convention fields directly simplifies usage.src/main/kotlin/BukkitGradlePlugin.kt (4)
9-14: Imports align with new plugin configuration.All newly introduced imports appear consistent and relevant to the renaming of
metatoplugin.
33-33: Extension creation is consistent with Gradle best practices.Using
extensions.create<BukkitExtension>to bind your new plugin configuration is a clean approach, keeping everything typed and centralized.
37-37: Good addition of thePluginConfigurationPlugin.Applying the new plugin for YAML parsing and merging ensures tasks are registered and available in the build lifecycle.
48-56: Conventions setup is clear and consistent.Defining standard properties such as
name,description,main,version, etc. ensures a sensible default experience. This matches Gradle’s recommended pattern for extension configuration.
src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/kotlin/plugin/PluginConfigurationPlugin.kt (3)
22-29: Check for project extension preconditions.
While thebukkitextension is typically guaranteed in this build setup, consider adding a short validation or an early fail ifbukkitis missing.
30-39: Safe-cast and clarify plugin type.
this.plugin = bukkit.plugin as PluginConfigurationImplis straightforward, but using a safe cast (as?) or an assertion can yield clearer error messages if something’s misconfigured.-this.plugin = bukkit.plugin as PluginConfigurationImpl +require(bukkit.plugin is PluginConfigurationImpl) { + "Expected 'bukkit.plugin' to be of type PluginConfigurationImpl but got something else." +} +this.plugin = bukkit.plugin as PluginConfigurationImpl
58-65: Resource directory lookup assumption.
val resourceDir = mainSourceSet.resources.srcDirs.first()will only consider the first resource directory. If multiple resource folders are used, it might ignore additional copies ofplugin.yml.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG.md(1 hunks)README.md(1 hunks)src/main/kotlin/Bukkit.kt(2 hunks)src/main/kotlin/BukkitExtension.kt(3 hunks)src/main/kotlin/BukkitGradlePlugin.kt(3 hunks)src/main/kotlin/dependencies/Dependencies.kt(1 hunks)src/main/kotlin/extensions/Accessors.kt(1 hunks)src/main/kotlin/meta/PluginMetaPlugin.kt(0 hunks)src/main/kotlin/meta/task/MergePluginMeta.kt(0 hunks)src/main/kotlin/meta/task/ParsePluginMetaFile.kt(0 hunks)src/main/kotlin/plugin/PluginConfigurationPlugin.kt(1 hunks)src/main/kotlin/plugin/PluginYaml.kt(3 hunks)src/main/kotlin/plugin/extension/PluginConfiguration.kt(1 hunks)src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt(2 hunks)src/main/kotlin/plugin/task/MergePluginYaml.kt(1 hunks)src/main/kotlin/plugin/task/ParsePluginYaml.kt(1 hunks)src/main/kotlin/plugin/util/MinecraftVersion.kt(1 hunks)src/main/kotlin/plugin/util/StringUtils.kt(1 hunks)src/main/kotlin/server/DevServerPlugin.kt(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy(11 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/plugin/util/StringUtilsSpec.groovy(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/kotlin/meta/task/MergePluginMeta.kt
- src/main/kotlin/meta/task/ParsePluginMetaFile.kt
- src/main/kotlin/meta/PluginMetaPlugin.kt
🚧 Files skipped from review as they are similar to previous changes (15)
- src/main/kotlin/extensions/Accessors.kt
- src/main/kotlin/plugin/util/MinecraftVersion.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/plugin/util/StringUtilsSpec.groovy
- src/main/kotlin/dependencies/Dependencies.kt
- src/main/kotlin/plugin/util/StringUtils.kt
- src/main/kotlin/server/DevServerPlugin.kt
- src/main/kotlin/plugin/task/ParsePluginYaml.kt
- src/main/kotlin/plugin/extension/PluginConfigurationImpl.kt
- CHANGELOG.md
- src/main/kotlin/plugin/PluginYaml.kt
- src/main/kotlin/Bukkit.kt
- src/main/kotlin/plugin/task/MergePluginYaml.kt
- README.md
- src/main/kotlin/plugin/extension/PluginConfiguration.kt
- src/main/kotlin/BukkitExtension.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/kotlin/BukkitGradlePlugin.kt (1)
src/main/kotlin/plugin/PluginConfigurationPlugin.kt (2)
apply(20-69)apply(22-56)
🔇 Additional comments (24)
src/main/kotlin/plugin/PluginConfigurationPlugin.kt (4)
20-21: Class naming is appropriate and consistent.
The namePluginConfigurationPluginconveys its purpose clearly, aligning with the newly renamed “plugin” terminology.
41-50: Parallel structure matches well.
Creating symmetrical tasks for parsing and merging plugin YAML fosters code clarity and maintainability.
66-68: Constant declaration looks good.
TheFILE_NAMEconstant is helpful for maintainability and consistency across tasks.
52-56:❓ Verification inconclusive
Duplicate strategy check.
duplicatesStrategy = DuplicatesStrategy.INCLUDEmight create multiple copies ofplugin.ymlif a file is already in the resources. Confirm that you do want duplicates merged in the final artifact.
🏁 Script executed:
#!/bin/bash # Checking for existing plugin.yml references in resources fd plugin.yml --exec cat {} | head -n 20Length of output: 44
Action Required: Verify and Confirm Duplicate Plugin.yml Handling
The current configuration sets
duplicatesStrategy = DuplicatesStrategy.INCLUDEwhich may cause duplicate copies of
plugin.ymlin the final artifact. The initial search usingfd plugin.yml --exec cat {} | head -n 20returned no output, so it’s unclear whether any duplicate
plugin.ymlfiles exist. Please manually verify that no unintended duplicates are introduced by running a command like:Then, confirm that merging duplicates is the desired behavior or adjust the configuration accordingly.
🏁 Script executed:
#!/bin/bash find . -type f -iname "plugin.yml"Length of output: 156
Action Required: Confirm Merging of Duplicate
plugin.ymlFilesThe configuration at
project.tasks.named<CopySpec>("processResources") { from(mergePluginYaml) duplicatesStrategy = DuplicatesStrategy.INCLUDE }means any duplicate
plugin.ymlfiles will be merged in the final artifact. We attempted to verify this with a script searching forplugin.yml, but the automated output was inconclusive due to a formatting issue with the execution tag.Please manually verify that there are no unintended multiple copies of
plugin.ymlin your resources by running a command such as:find . -type f -iname "plugin.yml"Confirm that merging duplicates is indeed the desired behavior. If duplicates aren’t present or are intentionally handled, then the current configuration is safe. Otherwise, consider adjusting the duplicate strategy.
src/main/kotlin/BukkitGradlePlugin.kt (4)
9-14: Imports align with the plugin rename.
The new references toPluginConfigurationPluginand its utilities reflect the shift from “meta” to “plugin.”
33-33: Merged extension creation is well-structured.
Creating thebukkitextension withPluginConfigurationImplandServerConfigurationImplin one call is clean and maintainable.
37-37: Explicit plugin application adds clarity.
ApplyingPluginConfigurationPluginexplicitly ensures the plugin tasks are set up.
48-49: Factory method for plugin configuration.
Encapsulating the creation ofPluginConfigurationImplin a dedicated function is neat and fosters testability and clarity.src/test/groovy/ru/endlesscode/bukkitgradle/plugin/task/MergePluginYamlSpec.groovy (16)
1-1: Package rename is consistent.
Renaming the package to match “plugin” instead of “meta” follows the overall refactor.
6-6: Test usage of PluginConfigurationPlugin.
ImportingPluginConfigurationPluginfor these specs aligns tests with the new plugin logic.
9-14: Additional fields track plugin YAML.
DeclaringsourcePluginYamlFileandpluginYamlFileclarifies the test scenarios for merging.
25-34: Validate “processResources” triggers.
The test ensuresmergePluginYamlandparsePluginYamlrun automatically, verifying the new plugin task dependency.
36-47: Disabled generation scenario.
Verifying skip logic ensures no accidental plugin YAML generation occurs.
50-56: Default plugin.yml generation test.
Checks the default config and ensures tasks produce the expected output.
63-79: API version tests.
Parameterizing the API version test is a solid approach to confirmbukkit.apiVersionlogic.
88-96: Idempotent task run.
Ensuring the second run is up-to-date is an important Gradle test pattern.
99-113: Configuration update detection.
Verifies the plugin re-merges when relevant fields change, reflecting the new system’s correctness.
122-127: Complete property coverage.
The test checks that all plugin properties are written toplugin.yamlin the new approach.
139-140: Ensuring success on processResources.
The test demonstrates that final output is correct after the merge.Also applies to: 143-144
154-166: Backward compatibility test.
Ensuring that the “old way” property assignments still produce correct plugin YAML is key to a smooth transition from meta to plugin.Also applies to: 171-183
185-201: Merge with extra source fields.
Confirms that existing YAML fields in the source file are carried over, ensuring no data loss in the new approach.Also applies to: 202-213
217-218: Exotic chars handling.
This scenario ensures non-ASCII text is respected, which is crucial for international plugin usage.Also applies to: 219-223, 229-229, 233-234
244-250: Source field precedence.
Validates that if fields are set only in source, they are preserved in the merged result.Also applies to: 252-252, 254-255, 256-260
263-268: Build script field precedence.
Checks that build script overrides source plugin fields for final plugin YAML.Also applies to: 270-278, 280-281, 283-284, 285-289
Closes #54