-
-
Notifications
You must be signed in to change notification settings - Fork 31
🐛 required controller make command should not be false #174
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
WalkthroughThe update modifies the way a user prompt is configured in the controller command logic. Specifically, it removes the explicit Changes
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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
🧹 Nitpick comments (1)
src/Console/Commands/ControllerMakeCommand.php (1)
160-160
: Fix grammatical error in prompt text.The prompt text contains a grammatical error: "What resource should this resource relies to?" should be corrected.
- 'What resource should this resource relies to? (Optional)', + 'What resource should this controller rely on? (Optional)',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Console/Commands/ControllerMakeCommand.php
(1 hunks)
🔇 Additional comments (1)
src/Console/Commands/ControllerMakeCommand.php (1)
159-162
:❓ Verification inconclusive
Verify that removing
required: false
doesn't change prompt behavior.The removal of the explicit
required: false
parameter could potentially change the behavior of the prompt if the default value for therequired
parameter in Laravel Prompts istrue
. Since the prompt text indicates "(Optional)" and the code expects a falsy value when no selection is made (line 164), this prompt should remain optional.
🏁 Script executed:
#!/bin/bash # Description: Check the default value of the 'required' parameter in Laravel Prompts select function # Expected: The default should be false to maintain optional behavior # Search for the select function definition in Laravel Prompts rg -A 10 "function select\(" --type phpLength of output: 41
Verify default
required
behavior and fix prompt grammarPlease confirm whether Laravel’s
select
prompt defaults torequired: false
. If the default istrue
, re-add therequired: false
argument to keep the prompt optional; otherwise you can leave it out. Also, there’s a grammatical issue in the prompt text—let’s rephrase it for clarity.• File:
src/Console/Commands/ControllerMakeCommand.php
Lines: 159–162Suggested changes (if you need to explicitly set
required: false
):- $resource = select( - 'What resource should this resource relies to? (Optional)', - $this->possibleResources() - ); + $resource = select( + 'Which resource should this new resource rely on? (Optional)', + $this->possibleResources(), + required: false + );Or, if
required
already defaults tofalse
, simply update the prompt text:- 'What resource should this resource relies to? (Optional)' + 'Which resource should this new resource rely on? (Optional)'
Summary by CodeRabbit