-
Notifications
You must be signed in to change notification settings - Fork 120
Make seeds a command argument. #935
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
| $continue = $io->askChoice('Do you want to continue?', ['y', 'n'], 'n'); | ||
| if ($continue !== 'y') { | ||
| $io->warning('Seed operation aborted.'); | ||
|
|
||
| return self::CODE_SUCCESS; |
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.
We could always ask for confirmation. If users don't want to deal with a confirmation prompt they can use the -q parameter or specify which seeds are to be run.
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.
Thats already how its done above with
if ($io->level() > ConsoleIo::QUIET) {
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.
I was suggesting that we don't have a confirm to go with the comment about having --all and no implicit run all. I get why you don't want to do that though.
| try { | ||
| $availableSeeds = $manager->getSeeds(); | ||
| } catch (Exception $e) { | ||
| $io->error('Failed to load seeds: ' . $e->getMessage()); | ||
|
|
||
| return static::CODE_ERROR; | ||
| } |
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.
I think that defaulting to running all seeds is a risky default, that should be an error instead. If there is a need for a 'run all' mode we could add a --all option.
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.
Thats why there is now a list of what seeds run and the confirmation before doing. The Extra option is not needed this way. You cannot a accidently do it anymore.
Seeds cleanup
Modernized the migrations seed command to use a cleaner argument-based syntax instead of options, added safety confirmations, and improved the overall user experience.
Key Improvements
When running all seeds without arguments, users now see: