Skip to content

Added resetCommand config - #85

Open
slischka wants to merge 1 commit into
nextras:masterfrom
slischka:84-reset-command-config
Open

Added resetCommand config#85
slischka wants to merge 1 commit into
nextras:masterfrom
slischka:84-reset-command-config

Conversation

@slischka

@slischka slischka commented Jul 29, 2018

Copy link
Copy Markdown

#84
BC break:

  • yes, you need enable in cofig to register reset command

missing:

  • Updated doc
  • Test where reset command is not registred

@Mikulas

Mikulas commented Jul 30, 2018

Copy link
Copy Markdown
Contributor

I'm against this change. The default configurator is fine as is. If you need different set of commands, you should register them into the symfony console app manually instead.

@slischka

Copy link
Copy Markdown
Author

It's about to disable reset command on production environment.
Yeah, there are a lot of way to delete production database. But I do not even want this comman enabled on production enviroment so i can not accidentally delete production DB via terminal/console.
The are other ways like you can disable or protect to not delete DB, like user permission. But some times you cannot set this think.
We can do it the other way, always enabled, but have a choise to disabled it.

And yes, I can setup this manualy. But why when i can just use configuration.

@jkuchar

jkuchar commented Jan 28, 2021

Copy link
Copy Markdown

I personally like this change, but I'm not sure it should "not register" command.

Wouldn't it be better to just from exception from "reset" command, expaining that reset has been disabled due to current configuration?

@dakur

dakur commented Jan 28, 2021

Copy link
Copy Markdown

I also like this change, especially the idea behind it – you never want to accidentally erase your database just because you mistyped or applied your habit from local development before you realize what you are doing. I don't mind the form pretty much – personally I just need to block me when I'm too quick – maybe the exception is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants