Chore: Cleanup invalid xml's from limetech repository #2475
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![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 (2)
emhttp/plugins/dynamix.docker.manager/include/DockerClient.php (2)
143-146: Consider using$dockerManPathsvariable for consistency.The path
/boot/config/plugins/dockerMan/templates/limetechis hardcoded, but the base path is already defined in$dockerManPaths['templates-usb']at line 30. For consistency with the rest of the codebase, consider using the variable.Apply this diff:
- /* Don't download any templates. Leave code in place for future reference. */ - /* remove existing limetech templates that are all not valid */ - exec("rm -rf /boot/config/plugins/dockerMan/templates/limetech"); - return []; + /* Don't download any templates. Leave code in place for future reference. */ + /* Remove existing limetech templates that are all not valid */ + global $dockerManPaths; + exec("rm -rf {$dockerManPaths['templates-usb']}/limetech"); + return [];
148-252: Consider removing unreachable code.The original template download logic (lines 148-252) is now unreachable due to the early return at line 146. While the comment mentions leaving it "for future reference," version control already preserves this code in the git history. Keeping 104 lines of dead code in the production file can cause confusion for future maintainers who may not immediately realize this code path is never executed.
If you'd like to preserve the implementation pattern for future reference, consider moving it to a separate documentation file or code comment rather than leaving it as unreachable code in the production file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix.docker.manager/include/DockerClient.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ljm42
Repo: unraid/webgui PR: 2414
File: etc/rc.d/rc.nginx:374-376
Timestamp: 2025-10-03T02:57:29.994Z
Learning: Repo unraid/webgui: In etc/rc.d/rc.nginx, maintainers prefer not to add explicit mv-failure checks or EXIT trap clearing around atomic writes in build_servers(), build_locations(), and build_ini(); treat mv failures (e.g., disk full/permissions) as non-recoverable and keep the implementation simple.
Learnt from: Squidly271
Repo: unraid/webgui PR: 2333
File: emhttp/plugins/dynamix/include/publish.php:28-36
Timestamp: 2025-08-13T03:13:53.283Z
Learning: In the unraid/webgui codebase, PR #2333 by Squidly271 supersedes earlier work that already addressed all legacy publish() calls using the old overloaded syntax. There are no remaining legacy calls that need backward compatibility handling in the publish() function.
Learnt from: zackspear
Repo: unraid/webgui PR: 2230
File: emhttp/plugins/dynamix/include/Templates.php:63-74
Timestamp: 2025-06-03T21:27:15.912Z
Learning: In the Unraid WebGUI codebase (emhttp/plugins/dynamix/include/Templates.php), there are known duplicate ID issues in checkbox templates across multiple template instances that the maintainers are aware of but have chosen not to address due to the effort required for legacy code improvements.
Learnt from: Squidly271
Repo: unraid/webgui PR: 2293
File: emhttp/plugins/dynamix.docker.manager/include/Helpers.php:302-304
Timestamp: 2025-07-11T17:24:33.977Z
Learning: In the Unraid WebGUI Docker manager (emhttp/plugins/dynamix.docker.manager/include/Helpers.php), the xmlSecurity() function call is intentionally commented out to allow users to include < and > characters in their Docker template variables on their own servers. Default templates still explicitly disallow tags, but users can customize templates as needed in their self-hosted environment.
Learnt from: Squidly271
Repo: unraid/webgui PR: 0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.
Learnt from: Squidly271
Repo: unraid/webgui PR: 0
File: :0-0
Timestamp: 2025-09-03T07:28:01.902Z
Learning: Squidly271's preference for not reviewing drafts is specifically because they find reviews annoying during active development phases with multiple commits, but they do want reviews once the PR is ready and more stable.
📚 Learning: 2025-07-11T17:24:33.977Z
Learnt from: Squidly271
Repo: unraid/webgui PR: 2293
File: emhttp/plugins/dynamix.docker.manager/include/Helpers.php:302-304
Timestamp: 2025-07-11T17:24:33.977Z
Learning: In the Unraid WebGUI Docker manager (emhttp/plugins/dynamix.docker.manager/include/Helpers.php), the xmlSecurity() function call is intentionally commented out to allow users to include < and > characters in their Docker template variables on their own servers. Default templates still explicitly disallow tags, but users can customize templates as needed in their self-hosted environment.
Applied to files:
emhttp/plugins/dynamix.docker.manager/include/DockerClient.php
3 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.