Use run-task plugin to run server #63
Conversation
There was a problem hiding this comment.
Copilot reviewed 27 out of 29 changed files in this pull request and generated no comments.
Files not reviewed (2)
- .editorconfig: Language not supported
- gradle.properties: Language not supported
Comments suppressed due to low confidence (1)
src/main/kotlin/server/util/Idea.kt:22
- The function findProjectDir returns a nullable File?, yet its value is passed directly to layout.dir which expects a non-null File. Consider adding a null-check or fallback value to prevent potential NullPointerExceptions.
val projectDir: Provider<Directory> = layout.dir(providers.provider { findProjectDir(layout.settingsDirectory.asFile) }) 
| @coderabbitai summary |
| Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you with the summary and review of this PR. Let me gather some information first. 🏁 Script executed: Length of output: 2070 Let me get more information about this PR to provide a proper summary and review. 🏁 Script executed: Length of output: 8092 Let me gather more information about the changes in this PR to provide a meaningful summary and review. 🏁 Script executed: Length of output: 2838 Let me continue gathering information about the changes in this PR, particularly focusing on the key files and dependencies that have been modified. 🏁 Script executed: Length of output: 22582 PR SummaryThis PR refactors how development servers are run in BukkitGradle by replacing the custom implementation with the established jpenilla/run-task plugin. Key changes include:
ReviewThe implementation looks good! Using an established plugin for server execution is a smart move that should reduce maintenance burden while providing better compatibility with various server implementations. A few observations:
Overall, this is a solid refactoring that improves maintainability without sacrificing functionality. The changes appear backward compatible while providing a cleaner, more modern implementation. |
| @coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis update modifies project configuration files, build scripts, and source code for the BukkitGradle plugin. Changes include refined Kotlin formatting settings, updated JVM version resolution and EULA command-line parameters, and a version bump. The server task registration has been restructured to remove outdated methods and deprecations, with a new run configuration for IntelliJ IDEA introduced. Additionally, several constants and obsolete classes are removed, and test cases have been updated to reflect the new control flow and parameter changes. Changes
Sequence Diagram(s)sequenceDiagram participant User participant Gradle participant DevServerPlugin participant RunServerTask User->>Gradle: Execute "prepareServer" task Gradle->>DevServerPlugin: Trigger task configuration DevServerPlugin->>RunServerTask: Register and configure "runServer" RunServerTask->>Gradle: Execute server with updated args (EULA accepted) Gradle->>User: Return server output sequenceDiagram participant User participant Gradle participant CreateIdeaGradleRunConfiguration participant IdeaUtil User->>Gradle: Run IDEA task (with idea.active true) Gradle->>CreateIdeaGradleRunConfiguration: Execute task CreateIdeaGradleRunConfiguration->>IdeaUtil: Build run configuration XML IdeaUtil-->>CreateIdeaGradleRunConfiguration: Provide configuration file path CreateIdeaGradleRunConfiguration->>User: Output generated XML configuration Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaGradleRunConfigurationSpec.groovy (1)
60-73: Test for custom configurations directory handles project-specific setupThis test correctly verifies that when users specify a custom configurations directory within the .idea folder, the task generates files with the right extension in the correct location.
Consider adding an additional test case where the configurations directory is outside the .idea folder to ensure broader coverage of possible configurations.
README.md (1)
38-38: Fix heading to remove trailing punctuation.According to MD026, remove the colon from the heading:
-#### TODO: +#### TODO🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
38-38: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
.editorconfig (1)
130-130: Import on-demand package configuration.Defining
ij_kotlin_packages_to_use_import_on_demandwith packages such asjava.util.*,kotlinx.android.synthetic.**, andio.ktor.**gives fine control over wildcard imports. Consider verifying that these are the only packages that should be imported on demand according to project conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.editorconfig(1 hunks)CHANGELOG.md(1 hunks)README.md(4 hunks)build.gradle.kts(2 hunks)gradle.properties(1 hunks)src/main/kotlin/BukkitGradlePlugin.kt(2 hunks)src/main/kotlin/extensions/Provider.kt(1 hunks)src/main/kotlin/meta/util/MinecraftVersion.kt(2 hunks)src/main/kotlin/server/Constants.kt(0 hunks)src/main/kotlin/server/DeprecatedServerProperties.kt(1 hunks)src/main/kotlin/server/DevServerPlugin.kt(2 hunks)src/main/kotlin/server/ServerProperties.kt(0 hunks)src/main/kotlin/server/extension/ServerConfiguration.kt(1 hunks)src/main/kotlin/server/extension/ServerConfigurationImpl.kt(3 hunks)src/main/kotlin/server/task/BuildServerCore.kt(0 hunks)src/main/kotlin/server/task/CreateIdeaGradleRunConfiguration.kt(1 hunks)src/main/kotlin/server/task/CreateIdeaJarRunConfiguration.kt(0 hunks)src/main/kotlin/server/task/DownloadPaperclip.kt(0 hunks)src/main/kotlin/server/task/PrepareServer.kt(0 hunks)src/main/kotlin/server/task/RunServer.kt(0 hunks)src/main/kotlin/server/util/Idea.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/server/extension/ServerConfigurationSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CopyServerCoreSpec.groovy(0 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaGradleRunConfigurationSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaJarRunConfigurationSpec.groovy(0 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/PrepareServerSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/util/IdeaSpec.groovy(1 hunks)
💤 Files with no reviewable changes (9)
- src/main/kotlin/server/task/RunServer.kt
- src/main/kotlin/server/ServerProperties.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CopyServerCoreSpec.groovy
- src/main/kotlin/server/task/PrepareServer.kt
- src/main/kotlin/server/task/CreateIdeaJarRunConfiguration.kt
- src/main/kotlin/server/Constants.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaJarRunConfigurationSpec.groovy
- src/main/kotlin/server/task/BuildServerCore.kt
- src/main/kotlin/server/task/DownloadPaperclip.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/kotlin/extensions/Provider.kt (3)
src/main/kotlin/server/DevServerPlugin.kt (1)
apply(33-64)src/main/kotlin/BukkitGradlePlugin.kt (2)
apply(18-80)apply(20-22)src/main/kotlin/meta/PluginMetaPlugin.kt (2)
apply(21-73)apply(23-59)
🪛 markdownlint-cli2 (0.17.2)
README.md
38-38: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (102)
src/test/groovy/ru/endlesscode/bukkitgradle/PluginSpecification.groovy (1)
80-82: Improved error reporting in test runs.Adding the
--stacktraceflag to all Gradle test runs will provide more detailed error information when tests fail, making debugging easier. This is a good practice for test utilities.gradle.properties (1)
3-3: Version bump to 0.11.0-SNAPSHOT is appropriate for these changes.The version increment from 0.10.1 to 0.11.0-SNAPSHOT correctly reflects the significant changes being introduced with the run-task plugin integration. This follows semantic versioning principles as these changes add new functionality.
src/main/kotlin/extensions/Provider.kt (2)
3-7: LGTM! Appropriate imports added for new functionality.These imports are necessary for the new directory provider function and properly organized.
11-11: Well-implemented utility function for directory configuration.This extension function provides a clean abstraction for creating directory providers, which will simplify task configuration in the DevServerPlugin. The implementation properly leverages Gradle's provider API for lazy configuration.
The function follows Gradle's best practices by:
- Using the provider pattern for lazy evaluation
- Utilizing the project's layout API for directory management
- Maintaining a functional approach with the lambda parameter
src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy (1)
23-29: Updated JVM version mappings align with Minecraft version requirements.The changes to JVM version mappings correctly reflect the minimal supported JVM versions for different Minecraft versions:
- 1.16.5 → JVM 8 (downgraded from 16)
- 1.17.1 → JVM 16
- New entries for 1.18 and 1.20.4 → JVM 17
- New entry for 1.20.5 → JVM 21
These updates align with the PR objective of using the minimal supported JVM version based on the Minecraft version.
CHANGELOG.md (2)
11-11: LGTM! Clear documentation of JVM toolchain versioning.This changelog entry effectively communicates the change to using the minimal supported JVM version based on the Minecraft version.
13-15: Well-documented EULA and debugging changes.The changelog clearly explains three important changes:
- EULA acceptance via CLI parameter instead of modifying eula.txt
- Changed default debug mode to false
- Recommendation to use IDE facilities for debugging
These entries provide users with the information they need to adapt to the changes.
src/main/kotlin/server/extension/ServerConfiguration.kt (1)
15-15: Method signature simplified for buildJvmArgsThe method signature no longer accepts a debug parameter, which aligns with the PR's goal of switching to the run-task plugin. This change makes the interface more straightforward but removes the flexibility to override debug settings per call.
The implementation now likely focuses on EULA acceptance via JVM args instead of debug configuration, which matches the PR objective of "allowing the acceptance of EULA via a CLI parameter."
src/main/kotlin/BukkitGradlePlugin.kt (2)
14-14: Improved import for JVM version resolutionThe import has been updated to use the new
resolveMinimalJavaVersionfunction instead of the removedresolveRecommendedJavaVersionfunction.
43-43: Switch to minimal JVM version requirementThe implementation now uses
resolveMinimalJavaVersioninstead ofresolveRecommendedJavaVersion, which aligns with the PR objective of updating "JVM toolchain handling to utilize the minimal supported JVM version based on the Minecraft version."This is a better approach as it provides more flexibility to developers who might want to use newer Java versions while ensuring compatibility with minimum requirements.
build.gradle.kts (3)
27-27: Added Gradle Plugin Portal repositoryAdding the Gradle Plugin Portal repository is necessary to fetch the jpenilla/run-paper plugin, which is a key component of this PR.
31-31: Integration of run-paper pluginThis is a core change that directly implements the PR's main objective of integrating the jpenilla/run-task plugin for server execution. Using this established plugin should reduce maintenance burden and improve compatibility with various server implementations.
55-55: Fixed VCS URL formatAppending
.gitto the VCS URL is a minor improvement that ensures the URL is in the proper format for Git repositories.src/test/groovy/ru/endlesscode/bukkitgradle/server/extension/ServerConfigurationSpec.groovy (1)
14-20: Updated test to verify EULA acceptanceThe test has been properly updated to verify the new EULA acceptance feature instead of debug flags. This change:
- Updates the test name to reflect its new purpose
- Sets the eula property to true before building JVM args
- Verifies that the EULA acceptance flag is included in the generated arguments
This aligns with the PR objective of adding EULA acceptance via CLI parameter and changing the default debug mode to false.
src/test/groovy/ru/endlesscode/bukkitgradle/server/util/IdeaSpec.groovy (2)
7-9: Method name and implementation updated to match refactored utility methodThe test method has been renamed from "test fileNameSlug" to "test sanitizeFileName" to match the new method name in the Idea utility class. This is a good practice to keep test names aligned with the methods they test.
15-18: Test cases updated for new filename sanitization behaviorThe test cases have been updated to reflect the new, less aggressive sanitization approach that preserves more of the original filename format. The new behavior:
- Preserves spaces (no longer converting to underscores)
- Only replaces colons with underscores
- Preserves brackets and their contents
This change likely improves readability of run configuration filenames in IDE.
src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaGradleRunConfigurationSpec.groovy (3)
1-14: Well-structured setup for IntelliJ run configuration testsThe test class provides a good foundation for testing the new IntelliJ IDEA run configuration generation functionality. The setup method correctly creates the required
.ideadirectory before each test.
15-29: Comprehensive tests for IDEA-based task execution scenariosThese two tests effectively verify the conditional execution behavior of the
:buildIdeaRuntask:
- The task runs successfully when the IDEA property is active
- The task is appropriately skipped when not run from IDEA
This is important for ensuring the task only generates configurations in the right context.
31-58: Thorough validation of generated XML configurationThis test effectively verifies that the XML configuration is correctly generated with the expected structure when run from IDEA. The configuration includes all essential elements:
- Project path
- Task to run (
runServer)- System settings and debug options
The XML structure matches the IntelliJ IDEA run configuration format.
src/main/kotlin/server/DeprecatedServerProperties.kt (4)
9-17: Well-designed deprecation handler for backward compatibilityThe
DeprecatedServerPropertiesclass is a clean approach to handling backward compatibility while encouraging migration to new property mechanisms. The class is properly scoped asinternaland has clear property definitions.
18-26: Eagerly loads properties to provide immediate deprecation warningsGood practice to eagerly load and check for deprecated properties during initialization, providing immediate feedback to users about properties they should migrate away from. This helps users identify outdated configurations early in the build process.
28-55: Clear deprecation messaging with helpful migration guidanceThe implementation of property resolution and deprecation warning is well-structured:
- Checks both local properties and environment variables
- Provides clear warning messages
- Includes specific guidance on how to replace deprecated properties
- Maintains backward compatibility by still creating directories if specified
This helps users migrate to the new system smoothly.
57-76: Well-encapsulated property definitions with clear migration pathsThe
Propertydata class is an elegant way to encapsulate all the information related to each deprecated property. The companion object provides clear constants and replacement notes for each property.For the
DEV_SERVER_DIRproperty, the replacement note points users to use the Gradle property system, which aligns with modern Gradle best practices.src/test/groovy/ru/endlesscode/bukkitgradle/server/task/PrepareServerSpec.groovy (3)
10-16: Test focus shifted to task success rather than dependenciesThe test now focuses on verifying that the
prepareServertask completes successfully, rather than explicitly checking dependency execution. This is more aligned with the integration of the run-task plugin mentioned in the PR objectives.
18-30: Fixed typo and simplified up-to-date testThe test description has been corrected from "should be uo-to-date" to "should be up-to-date", fixing a typo. The test execution has been simplified by removing unnecessary task exclusions, making the test more focused on the behavior being verified.
32-48: EULA handling removed from test, aligning with CLI parameter approachThe test has been updated to focus only on online-mode configuration, removing EULA-related assertions. This aligns with the PR objective of accepting the EULA via CLI parameter rather than file manipulation. The server directory path has also been updated to "run/" reflecting the new default location.
src/main/kotlin/meta/util/MinecraftVersion.kt (3)
3-3: Imports look good.Importing
JavaLanguageVersionis necessary for the new minimal Java version resolution logic.
27-27: Constant for 1.18 added correctly.
V1_18_0aligns with the existing format for version constants.
34-44: Ensure correctness of version-to-Java mappings.The mapping references the official wiki pages to determine the minimal Java version for each Minecraft version. This is accurate for current releases; however, future updates or new versions might require maintenance.
Please confirm these ranges are tested thoroughly in your integration or unit tests to guard against regression.
README.md (12)
4-6: Reworded description is clearer.This updated phrasing better conveys the plugin’s intent to help both writing and debugging Bukkit/Spigot plugins.
36-36: Reference to jpenilla/run-task is consistent with PR objectives.Highlighting the new chosen plugin for running the server makes the documentation more straightforward.
42-44: Noting Gradle 8.0+ is required.This update is in line with the plugin's migration to newer Gradle features.
200-201: Mentioning :runServer aligns with the new run-task plugin usage.Clear documentation for users to run the dev server via the Gradle task.
203-208: Instructions for building an IDEA run configuration.Helps users debug quickly from their IDE. Nicely documented.
217-218: Defaults for server location.Explicit mention of server location simplifies project setup.
220-221: Gradle property example is helpful.Guiding users to set
bukkitgradle.server.dirclarifies customizing the run directory.
225-230: Inline configuration snippet is user-friendly.Demonstrates setting the run directory directly in Gradle.
232-238: Tip about global Gradle property is useful.Allowing a shared dev server directory across multiple projects is a convenient feature.
241-261: Documenting EULA acceptance and server configuration.Matches the new approach for easily toggling these entries through Gradle properties.
265-267: Clarification on overriding eula/server properties.Explicit mention of precedence helps avoid confusion.
336-336: Linking to jpenilla/run-task for further reference.Convenient for users who want to explore the plugin’s repository.
src/main/kotlin/server/DevServerPlugin.kt (9)
6-19: New imports align with the plugin’s refactoring.They match the introduction of Java toolchain logic (
JavaToolchainService) and jpenilla’s run-paper integration (RunServer).
38-39: Applying RunPaperPlugin and resolving server directory.Cleanly sets up the plugin and checks for an overridable server directory.
41-53: Configuring runServer task.
- Uses
jvmArgs,args, and debug options, which neatly replicate prior functionality in a more maintainable way.- Clear separation of concerns with version and run directory.
56-58: Use of afterEvaluate.Necessary to finalize configuration after RunPaperPlugin sets its conventions.
60-64: Linking prepareServer and runServer tasks.Ensures any needed setup runs before the server is launched.
66-79: Default JVM configuration for server tasks.Automatically sets Java launcher using
resolveMinimalJavaVersion. This is consistent with the new approach inMinecraftVersion.kt.
81-89: resolveConfiguredServerDir fallback handling.Allows backward compatibility by checking the deprecated property.
92-95: Preparing the server with serverDir from runServer.Keeps the tasks aligned in terms of server location.
101-106: IDE run configuration generation.Provides a user-friendly method to build run configurations, referencing the runServer task directly.
src/main/kotlin/server/extension/ServerConfigurationImpl.kt (4)
14-14: Default debug mode off.Changing the default debug value to false is a reasonable move for production settings. Ensure that this does not unintentionally hide necessary debug output when users first configure the server.
18-18: Removal of default "nogui" argument.Using
emptyList()forbukkitArgsindicates no default Bukkit arguments. This is fine as long as the user is aware that they must specify any necessary arguments themselves.
51-54: Simplified EULA logic.Replacing debug logic with EULA acceptance flow using
listOfNotNullis clean. Consider verifying in the larger workflow whether EULA refusal overrides are needed.
70-70: Constant for EULA acceptance.
-Dcom.mojang.eula.agree=trueis standard for Mojang EULA acceptance, but double-check that future EULA changes do not require additional parameters.src/main/kotlin/server/util/Idea.kt (7)
3-9: New imports introduced.The imports for file/project layout, providers, and injection look appropriate.
11-14: Abstract class with injected dependencies.Defining an abstract class with
@Inject constructorto handle providers and layout is a clean, extensible pattern.
16-18: System property-based activation.Mapping
idea.activeto aProvider<Boolean>is a neat solution for toggling functionality without polluting code with environment checks.
20-28: Project directory detection logic.Traversing up the directory tree to locate
.idea/is helpful for automatically finding the project root. If a project lacks a.idea/folder, be mindful of null returns.
30-34: Relative path resolution.Creating a
$PROJECT_DIR$/...path mapping is convenient for IntelliJ. The use oftrimEnd('/')ensures consistent path formatting.
35-38: Run configuration file naming.Differentiating file extensions based on directory naming is clever. Mind that changes to
.idea/runConfigurationnaming conventions might break this in the future.
40-47: Companion object constants.Storing constants in a companion object is standard.
sanitizeFileNameeffectively handles invalid characters for Windows-based filesystems.src/main/kotlin/server/task/CreateIdeaGradleRunConfiguration.kt (7)
1-21: New task definition and imports.Declaring a custom Gradle task for generating run configurations is a good architectural separation. The Groovy and IDE-related imports look legitimate.
22-25: Class declaration and injection.Using a private constructor plus an internal one that injects
Ideaensures controlled instantiation while allowing Gradle's object factory usage.
27-44: Property definitions for run configuration.Each property is well-labeled as
@Inputor@Internalas needed. This approach ensures Gradle can track changes for UP-TO-DATE checks.
45-47: Configuration file provider.Combining
configurationsDirandconfigurationNamewithidea.runConfigurationFileis a neat pattern.
49-51: Secondary constructor usage.Delegating to the primary constructor ensures a consistent setup for
Idea.
53-65: Initialization logic andonlyIfpredicate.Conditionally running this task only when
idea.isActiveistrueprevents noise in non-IDE contexts. Defaults forvmOptionsandscriptParametersare helpful.
67-104: Creating the Gradle run configuration XML.The XML MarkupBuilder usage is standard. The attribute set for Gradle is correct, and the approach is well-structured.
.editorconfig (34)
24-28: Establish Kotlin-specific file settings.The new file pattern for Kotlin files (
[{*.kt,*.kts}]) correctly scopes the settings to Kotlin sources. Overriding the global continuation indent withij_kotlin_continuation_indent_size = 4(accompanied by the clarifying comment) and unsetting indents on empty lines aligns these rules with common Kotlin style guidelines (e.g. matching ktlint expectations).
30-56: Validate Kotlin spacing configurations.The detailed spacing rules for Kotlin files—covering spaces before parentheses (for
if,for,while,catch, andwhen), around operators, and for commas/colons/lambda arrows—are explicitly defined. This promotes consistency and code readability. Please ensure these settings align with your team’s agreed style.
58-59: Introduced section for wrapping and braces.The new header for wrapping and braces (with the “## Keep when reformatting” subheading) clarifies the intent to manage line breaks and brace positioning. It’s a useful addition for aligning IDE reformatting with project standards.
62-62: Extends/Implements list formatting.The header “## Extends/implements list” defines the context for settings that manage the wrapping of superclass or interface lists. This clear categorization supports readability in complex type declarations.
66-66: Function declaration parameters formatting.Defining a dedicated section for function declaration parameters (including wrapping, alignment, and newline behaviors) enhances clarity when viewing multiline function signatures.
72-72: Function call arguments formatting.The settings under “## Function call arguments” enforce consistent wrapping and alignment for function calls, which is particularly beneficial when calls span multiple lines.
78-78: Function parentheses alignment.This header groups configurations related to the layout of function parentheses. It ensures that parameter lists and method calls maintain a uniform appearance.
80-80: Chained function calls wrapping.Introducing settings for chained function calls helps maintain a neat layout in fluent APIs or builder patterns. This section clarifies how long call chains are to be wrapped.
84-84: If statement formatting guidelines.The dedicated header for
if()statement formatting indicates that specific rules (like newline placement forelseor condition formatting) are being enforced, contributing to consistent control flow appearance.
88-88: Do-while statement formatting.Even though
do ... while()loops are less common in Kotlin, providing explicit formatting settings for them shows thoroughness in addressing all language constructs.
90-90: Try-catch-finally formatting standards.The “## 'try' statement” section delineates the formatting for exception handling blocks. This ensures that the layout for
try,catch, andfinallyremains clear and consistent.
93-93: Binary expressions formatting.Introducing formatting rules for binary expressions helps in maintaining consistency in operator spacing, which is key for both clarity and maintainability.
95-95: General wrapping settings.This “## Wraps” section provides a centralized area for controlling various code-wrapping behaviors. Consolidating these settings makes future adjustments more straightforward.
103-106: When statements configuration.The settings for
whenstatements—ensuring a line break after multiline entries and proper indentation before arrows—improve readability for this commonly used Kotlin control structure.
107-107: Braces placement guidelines.Defining how braces should be placed (with the header “## Braces placement”) reinforces a consistent style throughout the code, which is vital for reducing merge conflicts and enhancing clarity.
109-109: Expression body functions settings.The header for expression body functions clarifies preferences for concise function notation. This helps streamline code that utilizes Kotlin’s expression body syntax.
112-112: Elvis expressions formatting.Setting preferences for the wrap behavior of Elvis expressions ensures that null-coalescing operations are both visually clear and consistent with the rest of the formatting rules.
116-116: Blank lines management.The introduction of a dedicated section for blank line control signals a proactive approach to managing vertical whitespace, which can enhance overall file organization.
117-117: Maximum blank lines control.By setting a maximum for blank lines in declarations and code, the configuration helps prevent excessive spacing that could detract from code compactness and readability.
121-121: Minimum blank lines configuration.Enforcing a minimum number of blank lines at strategic locations aids in visually separating logical code blocks, which can be especially useful for long or complex files.
126-126: Imports grouping settings.The header for the imports section, along with specific configuration for star import thresholds and layout rules, is a well-organized approach to manage dependencies and enhance clarity in import statements.
133-133: Other settings header.The “# Other” header helps in clearly segregating miscellaneous formatting rules from the more structured sections above.
134-134: Trailing comma settings.The configuration for trailing commas, including the explicit setting for call sites, is beneficial for cleaner diffs and easier code modifications, provided it aligns with the team’s style guide.
136-136: Trailing comma on call site permission.Setting
ij_kotlin_allow_trailing_comma_on_call_site = falseis a deliberate choice that may help avoid potential syntactic issues on certain platforms.
138-138: Code generation settings header.The “# Code generation” section starts a new grouping for settings that affect generated code. This separation aids in maintaining a clear structure for formatting guidelines that indirectly affect developer-written code.
139-139: Comment code settings.The configurations under “## Comment code” ensure that both line comments and block comments are formatted in a consistent manner, which contributes to overall documentation readability within the code.
142-142: Line comment reformatting.Disabling extra space on reformat for line comments (
ij_kotlin_line_comment_add_space_on_reformat = false) conforms to a stricter comment layout. It would be good to verify this behavior in your IDE to ensure it meets team standards.
144-144: Block comment space adjustment.Setting
ij_kotlin_block_comment_add_space = falsereinforces a uniform approach to comment styling. Confirm that this aligns with other documentation style rules within the project.
146-146: Load/Save settings header.The header for load/save configurations organizes settings related to the persistence of code style preferences, streamlining future configuration management.
147-147: Code style defaults update.Switching to
ij_kotlin_code_style_defaults = KOTLIN_OFFICIALensures adherence to the official Kotlin styling guidelines, which can enhance consistency across the codebase.
152-152: YAML file formatting settings.The new block for YAML files (
[{*.yaml,*.yml}]) introduces specific formatting rules, such as indentation size and blank line behavior, thereby extending the project’s style consistency to configuration files.
154-154: YAML indentation reset.Setting
ij_yaml_keep_indents_on_empty_lines = unsetharmonizes YAML formatting with the overall style strategy, reducing unwanted indentation artifacts.
156-156: YAML spaces within brackets.Disabling spaces within brackets in YAML files (
ij_yaml_spaces_within_brackets = false) helps ensure that the file structure remains clean and syntactically correct.
158-158: Shell script file formatting settings.The block for shell scripts (
[{*.bash,*.sh,*.zsh}]) with specified indent and tab width values ensures that shell scripts have a consistent and readable format across various environments.
| @coderabbitai could you add notes to the changelog for the changes I've missed there? |
| Tip For best results, initiate chat on the files or code changes. A summary of the edits CodeRabbit can apply:
A plan of each step that CodeRabbit will take in order to execute the edits for each file (with inline rough draft snippets): ➕ CHANGELOG.mdInsert the following changelog note before the "### Fixed" section (i.e. at line 15): - Refactor development server implementation to use [jpenilla/run-task](https://github.com/jpenilla/run-task/) plugin and integrate run-paper for server execution, improving maintainability and compatibility with various server versions. Review the updated CHANGELOG.md to ensure that the new note is clearly documented under the "Changed" section.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CHANGELOG.md (1)
16-16: Missing article in the EULA acceptance descriptionYou might be missing the article "the" in this line.
-Accept EULA using CLI parameter `-Dcom.mojang.eula.agree=true` instead of changing `eula.txt` +Accept the EULA using CLI parameter `-Dcom.mojang.eula.agree=true` instead of changing `eula.txt`🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: You might be missing the article “the” here.
Context: ...teMetaproperties. - Accept EULA using CLI parameter-Dcom.mojang.eula.agree=true...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
src/main/kotlin/server/DevServerPlugin.kt (1)
66-79: Consider adding error handling for missing JavaToolchainServiceThe method gracefully handles configuring the JVM toolchain based on Minecraft version, but it silently returns when
JavaToolchainServiceis not found.Consider adding a warning log message when the service is not available:
private fun Project.configureDefaultJvmForServer() { - val toolchains = project.extensions.findByType<JavaToolchainService>() ?: return + val toolchains = project.extensions.findByType<JavaToolchainService>() + if (toolchains == null) { + logger.warn("JavaToolchainService not found. Unable to configure default JVM for server.") + return + } val spec = the<JavaPluginExtension>().toolchain
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.editorconfig(1 hunks)CHANGELOG.md(1 hunks)README.md(4 hunks)build.gradle.kts(2 hunks)src/main/kotlin/BukkitGradlePlugin.kt(2 hunks)src/main/kotlin/extensions/Provider.kt(1 hunks)src/main/kotlin/meta/util/MinecraftVersion.kt(2 hunks)src/main/kotlin/server/Constants.kt(0 hunks)src/main/kotlin/server/DeprecatedServerProperties.kt(1 hunks)src/main/kotlin/server/DevServerPlugin.kt(2 hunks)src/main/kotlin/server/ServerProperties.kt(0 hunks)src/main/kotlin/server/extension/ServerConfiguration.kt(1 hunks)src/main/kotlin/server/extension/ServerConfigurationImpl.kt(3 hunks)src/main/kotlin/server/task/BuildServerCore.kt(0 hunks)src/main/kotlin/server/task/CreateIdeaGradleRunConfiguration.kt(1 hunks)src/main/kotlin/server/task/CreateIdeaJarRunConfiguration.kt(0 hunks)src/main/kotlin/server/task/DownloadPaperclip.kt(0 hunks)src/main/kotlin/server/task/PrepareServer.kt(0 hunks)src/main/kotlin/server/task/RunServer.kt(0 hunks)src/main/kotlin/server/util/Idea.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/server/extension/ServerConfigurationSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CopyServerCoreSpec.groovy(0 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaGradleRunConfigurationSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaJarRunConfigurationSpec.groovy(0 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/task/PrepareServerSpec.groovy(1 hunks)src/test/groovy/ru/endlesscode/bukkitgradle/server/util/IdeaSpec.groovy(1 hunks)
💤 Files with no reviewable changes (9)
- src/main/kotlin/server/task/PrepareServer.kt
- src/main/kotlin/server/Constants.kt
- src/main/kotlin/server/ServerProperties.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CopyServerCoreSpec.groovy
- src/main/kotlin/server/task/RunServer.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaJarRunConfigurationSpec.groovy
- src/main/kotlin/server/task/DownloadPaperclip.kt
- src/main/kotlin/server/task/CreateIdeaJarRunConfiguration.kt
- src/main/kotlin/server/task/BuildServerCore.kt
🚧 Files skipped from review as they are similar to previous changes (9)
- src/main/kotlin/server/extension/ServerConfiguration.kt
- src/main/kotlin/extensions/Provider.kt
- src/test/groovy/ru/endlesscode/bukkitgradle/BukkitGradlePluginSpec.groovy
- src/test/groovy/ru/endlesscode/bukkitgradle/PluginSpecification.groovy
- build.gradle.kts
- src/test/groovy/ru/endlesscode/bukkitgradle/server/task/CreateIdeaGradleRunConfigurationSpec.groovy
- src/main/kotlin/server/extension/ServerConfigurationImpl.kt
- src/main/kotlin/server/DeprecatedServerProperties.kt
- src/main/kotlin/server/task/CreateIdeaGradleRunConfiguration.kt
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[uncategorized] ~16-~16: You might be missing the article “the” here.
Context: ...teMetaproperties. - Accept EULA using CLI parameter-Dcom.mojang.eula.agree=true...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
README.md
[uncategorized] ~241-~241: You might be missing the article “the” here.
Context: ...one. ### Dev server configuration Use bukkit.server section to accept EULA an...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~241-~241: You might be missing the article “the” here.
Context: ... Use bukkit.server section to accept EULA and configure the server: ```groovy bu...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
README.md
38-38: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (40)
src/test/groovy/ru/endlesscode/bukkitgradle/server/task/PrepareServerSpec.groovy (6)
10-15: Test focus improved to align with new implementationThe test description and execution have been simplified to focus on the successful execution of the prepareServer task rather than its dependencies. This aligns with the PR objective of integrating the run-task plugin, which has likely changed how task dependencies are managed.
18-20: Fixed typo in test descriptionGood catch fixing the typo from "uo-to-date" to "up-to-date".
26-29: Simplified test executionRemoving the task exclusion simplifies the test and makes it more readable while still effectively testing the up-to-date behavior.
32-34: Refocused test to align with EULA changesThe test description has been updated to focus only on the online-mode configuration, removing the EULA testing. This aligns with the PR objective of handling EULA acceptance via CLI parameter rather than manual file editing.
41-41: Updated server directory pathThe server directory path has been changed from "build/server/1.16.2/" to "run/", which aligns with the run-task plugin's default directory structure.
44-47: Simplified assertionsThe test now only asserts the online-mode setting in server.properties, removing EULA-related assertions. This is consistent with the PR objective of changing EULA handling.
src/main/kotlin/server/util/Idea.kt (6)
11-14: Well-designed conversion from object to abstract class with dependency injectionThe refactoring of
Ideafrom an object to an abstract class with dependency injection is a good architectural change. This approach aligns with modern Gradle practices and allows for proper testability by injecting dependencies.
16-18: Proper usage of Provider API for lazy configurationExcellent use of the Provider API to lazily evaluate the system property and provide a default value. This approach is more efficient and follows Gradle's recommended patterns for handling properties.
20-28: Well-implemented recursive directory searchThe implementation of
findProjectDiras a tail-recursive function is efficient and elegant. The function correctly handles the edge case when the parent directory is null by returning null in that case.
30-33: Clear implementation of relative path resolutionThe
relativePathOfmethod is well-implemented, providing a clean way to get the relative path from the project directory. Using the IntelliJ-specific variable$PROJECT_DIR$is appropriate for this context.
35-38: Smart run configuration file handlingThe
runConfigurationFilemethod intelligently determines the appropriate file extension based on the directory path, making it adaptable to different IntelliJ IDEA versions. Good use of thesanitizeFileNamemethod to ensure valid filenames.
40-47: Well-organized companion object with clear constants and utility methodsThe companion object is well-structured with clear constants and a utility method for sanitizing file names. The use of a regex to identify characters that need replacement is an efficient approach.
src/main/kotlin/BukkitGradlePlugin.kt (2)
14-14: Appropriate change to use minimal Java versionGood change to import and use
resolveMinimalJavaVersioninstead ofresolveRecommendedJavaVersion. This aligns with the PR objective of using the minimal supported JVM version based on the Minecraft version.
43-43: Improved Java version resolutionThe switch from
resolveRecommendedJavaVersiontoresolveMinimalJavaVersionensures that the plugin uses the minimal required Java version for compatibility, rather than an unnecessarily high version. This is more flexible and user-friendly.CHANGELOG.md (3)
9-11: Well-documented breaking changeThe breaking change is clearly marked and well-explained, providing users with detailed information about the refactoring of the development server implementation to use the jpenilla/run-task plugin.
14-14: Improved JVM toolchain configuration explainedGood explanation of how the JVM toolchain version is now determined based on the minimal supported JVM version compatible with the specified Minecraft version.
17-18: Clear explanation of debug mode changeGood explanation of the change to the default value of
bukkit.server.debugand the recommendation to use IDE facilities for debugging instead.src/test/groovy/ru/endlesscode/bukkitgradle/server/util/IdeaSpec.groovy (1)
7-19: Updated test method name and test cases for new sanitization approachThe test has been correctly updated to reflect the changes in the
Ideaclass. The method name now appropriately refers tosanitizeFileNameinstead offileNameSlug, and the test cases have been updated to match the new behavior.src/test/groovy/ru/endlesscode/bukkitgradle/server/extension/ServerConfigurationSpec.groovy (1)
14-20: Test updated to verify EULA flag inclusion instead of debug flagsThe test case has been successfully updated to verify that when
eulais set to true, the JVM arguments include the EULA acceptance flag. This is consistent with the PR's objective to allow EULA acceptance via CLI parameter.src/main/kotlin/server/DevServerPlugin.kt (4)
38-64: Well-implemented integration with RunPaperPluginThe implementation successfully integrates the
jpenilla/run-taskplugin for server execution, replacing the custom implementation. TherunServertask is properly configured with server version, directory, JVM/Bukkit args, encoding, and debug options.
81-89: Clear handling of configured server directoryThe method effectively resolves the server directory from Gradle properties, with proper fallback to deprecated properties and null handling.
91-99: Updated task dependency structureThe
prepareServertask is now properly associated with therunServertask instead of being tied to specific server directories or copy tasks, creating a more logical dependency flow.
101-106: Enhanced IDE run configurationThe
buildIdeaRuntask is now correctly configured to reference therunServertask directly, which improves integration with IDEs.src/main/kotlin/meta/util/MinecraftVersion.kt (1)
34-44: Well-implemented minimal Java version resolutionThe new functions for determining minimal Java version requirements are well-structured and include clear references to Minecraft wiki documentation. The implementation properly handles all relevant Minecraft version ranges and their corresponding Java requirements.
README.md (4)
5-6: Updated project descriptionThe description has been improved to emphasize both writing and debugging aspects of the plugin.
36-37: Updated feature descriptionFeature description has been updated to reflect the use of
jpenilla/run-taskinstead of the previous custom implementation.
200-238: Comprehensive updated server run instructionsThe instructions for running the dev server have been thoroughly updated to reflect the new implementation. The section provides clear guidance on:
- Using the
:runServertask- Creating IDEA run configurations
- Configuring the server directory
- Sharing server directories between projects
This update greatly improves user experience with the refactored implementation.
241-268: Clear server configuration documentationThe documentation for server configuration has been updated to accurately reflect the new approach to EULA acceptance and server configuration. The note about precedence of
eulaandonline-modeoptions over file-based settings is especially helpful.🧰 Tools
🪛 LanguageTool
[uncategorized] ~241-~241: You might be missing the article “the” here.
Context: ...one. ### Dev server configuration Usebukkit.serversection to accept EULA an...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~241-~241: You might be missing the article “the” here.
Context: ... Usebukkit.serversection to accept EULA and configure the server: ```groovy bu...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
.editorconfig (12)
24-28: Kotlin File-Specific Settings Added
A new configuration block for Kotlin files ([{*.kt,*.kts}]) has been introduced. The settings for continuation indent size and handling of indents on empty lines (set tounset) appear to be designed to align with ktlint or the team's coding standards. Please verify that these settings provide the desired behavior in your IDE.
30-57: Comprehensive Spacing and Operator Rules Configuration
This segment defines detailed rules for spacing before parentheses (e.g., forif,for,while, etc.), around various operators, commas, type colons, and lambda arrows. These settings ensure consistent formatting across Kotlin files. Double-check that these rules do not conflict with any formatter defaults in your development environment.
58-115: Consistent Wrapping, Parentheses, and Braces Formatting
The configuration for wrapping elements—covering function declarations, call arguments, chained calls, control statements, and binary expressions—is clearly laid out. These rules mirror standard Kotlin style guidelines. It’s advisable to confirm that these settings integrate nicely with IDE defaults and do not cause unexpected reformatting issues.
116-125: Blank Lines Configuration
The rules for maintaining maximum and minimum blank lines in different code contexts are straightforward and should help maintain clean code structure. No issues detected; these settings align with general formatting standards.
126-132: Import Statements Formatting
The import configuration specifies thresholds for using star imports as well as the desired import layout. These settings contribute to maintaining a clean and consistent import structure. Ensure that these patterns match the project’s overall style guide for imports.
133-137: Trailing Comma and Other Miscellaneous Rules
This short segment governs the behavior for trailing commas—allowing them generally but disabling them on call sites. Confirm that this behavior is intentional for your code style and that it meshes well with both manual and automated formatting processes.
138-145: Code Generation and Comment Formatting Settings
The updated settings for code comments (both line and block) and their spacing ensure that generated code remains uniform. These settings should help maintain both readability and consistency across files.
146-148: Load/Save Default Code Style
The configuration now setsij_kotlin_code_style_defaultstoKOTLIN_OFFICIAL, which is a robust default for enforcing Kotlin’s recommended styling. Make sure that this selection is compatible with your team’s preferred style and any integrated tools.
152-157: YAML Formatting Settings Updated
The section for YAML files specifies an indent size of 2 and adjusts the handling of indents on empty lines and spaces within brackets. These rules are in line with common YAML formatting practices. Verify that they meet your project's requirements.
158-161: Shell Script Formatting Settings
The configuration for bash/sh/zsh files is set to an indent size of 2 with a tab width of 2. This should ensure a consistent format in shell scripts.
162-164: Batch File End-of-Line Setting
For batch files, the end-of-line is set tocrlf, which is standard for Windows environments. This change is both appropriate and expected.
165-167: Properties File Formatting
The properties file configuration preserves blank lines as specified. This minimal configuration is acceptable and consistent with typical properties file formatting needs.
Breaking change!
Refactor development server implementation to use jpenilla/run-task plugin and integrate run-paper for server execution,
improving maintainability and compatibility with various server versions.
false