Closed bronco0 closed 7 years ago
@bronco0 Thanks for your pull request and sorry for delay. I have some questions.
1- Why are your commite messages not in english? Please change them to English 2- I did not get why you created a new config file and why you changed threshold to 1? 3- I blieve the config for this new feature should change. That's a horrible looking config to me. And to be more percise we should discuss about how to implement this feature. For instance, shouldn't we have a global view for disabled actions instead of only echoing something and die? Isn't it better to have a helper function to check if the action is enabled or not (Instead of checking config again and again)? Or as I mentioned before, what should be the structure of the config for this?
Hi,
1) Sorry, I am french ! Yes, I'll change to English. 2) I don't create a new config file. I just created a copy of original config file to sample file. When I clone my PHPRemin (fork), I copy config.sample.dist.php to config.dist.php and edit this new file. Config.dist.php is in the .gitignore file, when I push the changes in GitHub, my setting is not pushed. 3) Yes, I also believe that this is not the best way to add these features, but I had to add these functionalities quickly. For me, the solution would be to add a accounts management and roles with acl. But have the choice of authentication method or not in the config.dist.php. Ex:
It's a very big job. Can one make my changes and then see to improve this ?
Thanks
@bronco0 Really sorry for the delay. As I mentioned before, I believe this is not the best way to implement this. This is adding a lot of complexity and dirty hacking to the code (which is already complicated and not clean) to add a feature for a use-case that you need immediately and is specific to you. I do not see any point for this much of control over enabling and disabling the features. I believe if someone really needs this for a security reason should consider disabling commands in Redis itself (https://redis.io/topics/security) As a result I prefer not to accept this pull request. But if you really believe this is a good to have feature can you raise a separate issue so we can discuss in details how to implement this? Thanks.
Added ability to enable or disable features, all can be configured in the configuration file.
For example:
Changes are made to the views and controllers.
I also replaced the config.dist.php file with config.sample.dist.php and added the config.dist.php file to the gitignore to keep a sample configuration without having problems with git when configure this application.
I have removed the .htaccess file but this change is not necessarily to take with my request.
Thanks