search

craue / CraueConfigBundle

Database-stored settings made available via a service for your Symfony project.
MIT License
173 stars 35 forks source link

Created a command to create configurations via the CLI #38

Closed bvisonl closed 5 years ago

bvisonl commented 6 years ago

Just a simple create command, I saw someone on the issues that was asking for this and I needed it myself as well. I am aware that it may seem like more trouble but when you have a bunch of settings and you just want to create a new one instead of having to touch the database it can be done through here and avoid a potential risk.

Also, an important note is that if someone has a custom entity this wouldn't work properly.

Let me know what you think!

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-11.03%) to 86.496% when pulling 50ccbf49ca81925e30f3f8c67b14fffe71e8b9b0 on bvisonl:create_command into eb0a0b7387ffaecd3fd261089e9c06224862a8ce on craue:master.

craue commented 6 years ago

Thank you for taking the time to contribute, but I have some concerns about this:

  1. You say that you'd like to use the command "instead of having to touch the database", but in fact, it's touching the database as well. 😏
  2. When running an app in several environments, it's more likely to forget creating settings by command calls rather than using versioned database migrations. This is my main concern/reason for not adding such a command. Furthermore, the point of #14 was to let a command generate a migration (instead of persisting the setting directly).
  3. It also has to work with a custom entity.
  4. Tests need to be added.
bvisonl commented 6 years ago

Understood your point.

  1. What I meant with the database interaction was that the person in charge wouldn't have to actually dial into the database and execute the insert (assuming it's not being done with a migration) and instead would be done through the shell.
  2. Also, I agree with the migration process for the configuration creation... However, it doesn't hurt to ALSO have the command in place. And also, depending on your deployment process and the way that you handle your multiple servers' configurations you could include this command to be executed on all of the servers as well.

+1 for the test, completely forgot about that, if you are still interested in this let me know and I will proceed to make the test and also make sure it works with custom configurations... I like the bundle, and I have a lot of use for it. Nice work and keep it up!

alexsegura commented 6 years ago

Hello,

I second this. This bundle is really useful (crazy this seems to be the only one out there to store settings in database...) BUT not being able to create settings programmatically or via CLI makes some simple things really complicated.

@craue I don't understand how you manage fresh installations when you say "use migrations"? Consider the following use case:

When creating a new instance of some app, migrations wont be executed: I would have used doctrine:migrations:version --add --all, because the code and the schema is already at the latest version.

Are you suggesting to create a migration which is dedicated to create the initial settings? Like Version000000000000? This seems like a hack...

Looks like @bvisonl has deleted his fork, I can make another pull request for this.

alexsegura commented 6 years ago

You can take a look at my implementation of the command here