update: dockerImage on Minecraft#2
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Warning Review limit reached
More reviews will be available in 57 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughMoves Docker image configuration from a hardcoded mapping in install.ts into a new ChangesDocker Image Configuration Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft/install.ts (1)
5-16:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore the removed
dockerImageexport or update the caller.
minecraft/index.ts, Line 3 still importsdockerImagefrom./install, so this refactor leaves the module in a typecheck-failing state. The current pipeline is already failing on that missing export.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft/install.ts` around lines 5 - 16, The export for dockerImage was removed but is still imported by minecraft/index.ts causing type errors; either reintroduce and export the dockerImage constant from this module (e.g., restore a dockerImage symbol alongside buildMinecraftCompose) or update the importer in minecraft/index.ts to use the new API (pass docker image via settings or read it from buildMinecraftCompose arguments). Ensure the symbol name dockerImage is present or change the import to the new property so TypeScript imports resolve.
🧹 Nitpick comments (1)
minecraft/settings.ts (1)
8-15: ⚡ Quick winAdd readable labels to the Docker image options.
This selector will otherwise show raw image URLs in the UI, which makes the new setting harder to use than the other Minecraft selects.
Proposed diff
options: [ - { value: "ghcr.io/pterodactyl/yolks:java_25" }, - { value: "ghcr.io/pterodactyl/yolks:java_22" }, - { value: "ghcr.io/pterodactyl/yolks:java_21" }, - { value: "ghcr.io/pterodactyl/yolks:java_17" }, - { value: "ghcr.io/pterodactyl/yolks:java_16" }, - { value: "ghcr.io/pterodactyl/yolks:java_11" }, - { value: "ghcr.io/pterodactyl/yolks:java_8" }, + { label: "Java 25", value: "ghcr.io/pterodactyl/yolks:java_25" }, + { label: "Java 22", value: "ghcr.io/pterodactyl/yolks:java_22" }, + { label: "Java 21", value: "ghcr.io/pterodactyl/yolks:java_21" }, + { label: "Java 17", value: "ghcr.io/pterodactyl/yolks:java_17" }, + { label: "Java 16", value: "ghcr.io/pterodactyl/yolks:java_16" }, + { label: "Java 11", value: "ghcr.io/pterodactyl/yolks:java_11" }, + { label: "Java 8", value: "ghcr.io/pterodactyl/yolks:java_8" }, ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft/settings.ts` around lines 8 - 15, The options array in minecraft/settings.ts currently supplies only raw image URLs as { value: ... }, which shows ugly strings in the UI; update each entry in the options array (the same array that lists ghcr.io/pterodactyl/yolks:java_*) to include a human-readable label field (e.g., { value: "ghcr.io/pterodactyl/yolks:java_17", label: "Yolks — Java 17" } or similar such as "Java 17 (Yolks)"), ensuring every object has both value and label so the selector displays friendly names instead of full image URLs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@minecraft/install.ts`:
- Around line 5-16: The export for dockerImage was removed but is still imported
by minecraft/index.ts causing type errors; either reintroduce and export the
dockerImage constant from this module (e.g., restore a dockerImage symbol
alongside buildMinecraftCompose) or update the importer in minecraft/index.ts to
use the new API (pass docker image via settings or read it from
buildMinecraftCompose arguments). Ensure the symbol name dockerImage is present
or change the import to the new property so TypeScript imports resolve.
---
Nitpick comments:
In `@minecraft/settings.ts`:
- Around line 8-15: The options array in minecraft/settings.ts currently
supplies only raw image URLs as { value: ... }, which shows ugly strings in the
UI; update each entry in the options array (the same array that lists
ghcr.io/pterodactyl/yolks:java_*) to include a human-readable label field (e.g.,
{ value: "ghcr.io/pterodactyl/yolks:java_17", label: "Yolks — Java 17" } or
similar such as "Java 17 (Yolks)"), ensuring every object has both value and
label so the selector displays friendly names instead of full image URLs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae5b3954-bccf-47a7-bccf-3495899d587f
📒 Files selected for processing (2)
minecraft/install.tsminecraft/settings.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft/index.ts (1)
9-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore
dockerImagerecipe field (or this PR cannot pass validation).CI is currently failing because the recipe validator still requires a
dockerImagefield inminecraft/index.ts. Even though Compose now correctly usessettings.dockerImage, this object shape no longer matches the enforced schema.Compatibility fix (until validator contract is updated)
export const minecraft = { buildCompose, + dockerImage: minecraftSettings.dockerImage.default, description: "Minecraft is a sandbox game where you can build your own world.", enabled: true,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft/index.ts` around lines 9 - 38, The exported minecraft object is missing the required dockerImage recipe field expected by the validator; add a dockerImage property to the minecraft export (alongside settings and buildCompose) and set it to the same value used in settings.dockerImage (or a sensible default string) so the object shape matches the validator contract; update the minecraft constant to include dockerImage while leaving settings.dockerImage and buildCompose unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@minecraft/index.ts`:
- Around line 9-38: The exported minecraft object is missing the required
dockerImage recipe field expected by the validator; add a dockerImage property
to the minecraft export (alongside settings and buildCompose) and set it to the
same value used in settings.dockerImage (or a sensible default string) so the
object shape matches the validator contract; update the minecraft constant to
include dockerImage while leaving settings.dockerImage and buildCompose
unchanged.
1 participant
Description
Please provide a brief description of the changes introduced in this pull request.
Related Issues
Closes #<issue_number>
Checklist
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit