feat(loader): add support for Purpur, Folia and more#97
feat(loader): add support for Purpur, Folia and more#97KinglyShade wants to merge 10 commits intopelican-dev:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactored MinecraftLoader enum: added cases (Purpur, Folia, Pufferfish, Spigot, Bukkit, Waterfall), replaced nested conditionals in fromTags with a match expression and an early guard, adjusted fromServer/fromTags return types to ?self, and switched getLabel() to use Str::title($this->name). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@minecraft-modrinth/src/Enums/MinecraftLoader.php`:
- Around line 38-58: Pint is flagging a missing blank line before the
return/match statement in the MinecraftLoader enum; add a single blank line
immediately before the "return match (true) {" statement (i.e., ensure there's
an empty line between the preceding if-block and the return match) or simply run
PHP Pint's auto-fix to apply the "blank_line_before_statement" rule, then re-run
tests; target the return match in the method that checks in_array('minecraft',
$tags) inside the MinecraftLoader enum.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
minecraft-modrinth/src/Enums/MinecraftLoader.php
🧰 Additional context used
🪛 GitHub Actions: Lint
minecraft-modrinth/src/Enums/MinecraftLoader.php
[error] 1-1: Pint: style issue 'blank_line_before_statem…'. Fix style issue detected by 'vendor/bin/pint --test'.
🔇 Additional comments (1)
minecraft-modrinth/src/Enums/MinecraftLoader.php (1)
15-22: Nice coverage for additional loaders.Enum expansion cleanly matches the PR objective and keeps the type surface area consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Removed enum cases and the getLabel method from MinecraftLoader.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@minecraft-modrinth/src/Enums/MinecraftLoader.php`:
- Around line 17-38: The file references enum MinecraftLoader and its cases
(NeoForge, Forge, Fabric, Quilt, Folia, Purpur, Pufferfish, Paper, Spigot,
Bukkit, Velocity, Waterfall, Bungeecord) in the match expression but the enum
declaration was removed; restore a backed enum MinecraftLoader with those named
cases (match the pattern used in ModrinthProjectType.php), ensuring the enum
name is MinecraftLoader and include all listed cases so the static method(s)
that return self::CaseName compile and run without fatal errors.
🧹 Nitpick comments (1)
minecraft-modrinth/src/Enums/MinecraftLoader.php (1)
16-35: Consider using English for comments and strictin_arraycomparison.Two minor suggestions:
- Comments in Spanish (lines 16, 24, 32): If the codebase convention is English, consider translating for consistency.
- Strict comparison: Using
in_array($tag, $tags, true)ensures type-safe matching.♻️ Optional: English comments and strict comparison
- // Mapeo de Tags a Enums + // Map tags to enum cases return match (true) { // Mods - in_array('neoforge', $tags) || in_array('neoforged', $tags) => self::NeoForge, - in_array('forge', $tags) => self::Forge, - in_array('fabric', $tags) => self::Fabric, - in_array('quilt', $tags) => self::Quilt, + in_array('neoforge', $tags, true) || in_array('neoforged', $tags, true) => self::NeoForge, + in_array('forge', $tags, true) => self::Forge, + in_array('fabric', $tags, true) => self::Fabric, + in_array('quilt', $tags, true) => self::Quilt, - // Plugins (Forks de Paper/Spigot) - in_array('folia', $tags) => self::Folia, + // Plugins (Paper/Spigot forks) + in_array('folia', $tags, true) => self::Folia, // ... apply similar changes to remaining in_array calls
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
minecraft-modrinth/src/Enums/MinecraftLoader.php
🔇 Additional comments (2)
minecraft-modrinth/src/Enums/MinecraftLoader.php (2)
1-7: LGTM!The
fromServermethod correctly lazy-loads the egg relationship and delegates tag resolution tofromTags.
12-14: Good early guard for the 'minecraft' tag.The early return improves readability and prevents unnecessary match evaluation for non-Minecraft servers.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Refactor MinecraftLoader enum to implement HasLabel interface and update methods.
Removed unnecessary comments and improved formatting.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@minecraft-modrinth/src/Enums/MinecraftLoader.php`:
- Around line 34-35: The PHPDoc for the local variable $tags in MinecraftLoader
(the comment "/** `@var` string[] $tags */") has extra spaces and fails Pint's
phpdoc_single_line_var_spacing rule; update the annotation to use single spaces
so it reads "/** `@var` string[] $tags */" (i.e., one space after `@var`, no extra
spaces before/after the type/name) to satisfy the linter.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
minecraft-modrinth/src/Enums/MinecraftLoader.php
🧰 Additional context used
🧬 Code graph analysis (1)
minecraft-modrinth/src/Enums/MinecraftLoader.php (2)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (1)
fromServer(29-45)billing/src/Models/Product.php (1)
egg(89-92)
🪛 GitHub Actions: Lint
minecraft-modrinth/src/Enums/MinecraftLoader.php
[error] 1-1: Pint style check failed. 1 style issue detected (phpdoc_single_line_var_s…). Fix the PHPDoc for a single-line var annotation in this file. Command: 'vendor/bin/pint --test'.
🔇 Additional comments (2)
minecraft-modrinth/src/Enums/MinecraftLoader.php (2)
9-23: LGTM!The expanded enum cases correctly cover the Minecraft server fork hierarchy (Bukkit → Spigot → Paper → Pufferfish/Purpur/Folia) and proxy variants. The string-backed values follow consistent lowercase convention.
43-67: LGTM!The
match(true)refactor improves readability. The ordering correctly prioritizes specific forks over their parents (e.g., Folia/Purpur/Pufferfish before Paper), ensuring the most specific loader is returned when multiple related tags exist. The tag aliases (neoforged,papermc,spigotmc,bungee) are a good addition for compatibility.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Updated PHPDoc for tags variable to improve clarity.
|
Thanks but those extra loaders aren't needed, they are basically just aliases for other loaders. Adding them as separate thing only makes the whole construct more complicated for no reason. |
This PR expands the MinecraftLoader enum to support additional popular server forks and proxies. This allows the Minecraft Modrinth plugin to correctly identify the server type and filter compatible plugins/mods from Modrinth more accurately.
Changes
Summary by CodeRabbit
New Features
Bug Fixes / Improvements