-
Notifications
You must be signed in to change notification settings - Fork 72
build #1833
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughUpdated the Mercator core dependency version, removed several Artisan module-management commands, switched a Mercator service provider entry in config, bumped package/version files, and added uninstall confirmation translations in English and French. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Console/Kernel.php (1)
13-18: Add CHANGELOG entry documenting the removal of module management Artisan commands.The removal of the four module management commands (mercator:module-install, mercator:module-enable, mercator:module-disable, mercator:module-uninstall) is well-architected: no external references exist, and full UI-based alternative functionality is already implemented in ModuleController with corresponding routes and views. However, this breaking change is not documented in CHANGELOG.md. Add an entry explaining that users should use the web UI for module management going forward.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
app/Console/Commands/MercatorModuleDisable.phpapp/Console/Commands/MercatorModuleEnable.phpapp/Console/Commands/MercatorModuleInstall.phpapp/Console/Commands/MercatorModuleUninstall.phpapp/Console/Commands/MercatorModulesList.phpapp/Console/Kernel.phpcomposer.jsonconfig/app.phpresources/lang/en/cruds.phpresources/lang/fr/cruds.php
💤 Files with no reviewable changes (5)
- app/Console/Commands/MercatorModuleEnable.php
- app/Console/Commands/MercatorModuleInstall.php
- app/Console/Commands/MercatorModuleDisable.php
- app/Console/Commands/MercatorModuleUninstall.php
- app/Console/Commands/MercatorModulesList.php
🚧 Files skipped from review as they are similar to previous changes (1)
- composer.json
🔇 Additional comments (3)
resources/lang/fr/cruds.php (1)
1017-1018: LGTM!The French translation for the module uninstall confirmation is correctly added and grammatically accurate.
resources/lang/en/cruds.php (1)
964-965: LGTM!The English translation for the module uninstall confirmation is clear and properly structured.
config/app.php (1)
235-235: [rewritten review comment]
[exactly one classification tag]
Summary by CodeRabbit
Chores
Removed
Config
Localization
✏️ Tip: You can customize this high-level summary in your review settings.