@czolnowski opened this Pull Request on November 6th 2014 Contributor

I have simply change const into config value and describe them in global.ini.php

@tsteur commented on November 6th 2014 Member

We really need to find a way to let plugins configure things without having to edit global.ini.php which should ideally not be used for this. It adds a dependency from core to a plugin but all the logic should be contained in the plugin otherwise we end up having many problems. Maybe in this case the Settings API could be used?

Of course having everything in one config file is nice for the users. Maybe plugins could also provide their own "global.ini.php" which could be merged somehow with our "global.ini.php". "somehow" :) Or something similar...
Edit: I am going to create an issue for this...

@czolnowski commented on November 6th 2014 Contributor

Settings API sounds very good for me! I'll try to write some code to solve this and attach to this PR, so please, hold with merging. :)

@mattab commented on November 7th 2014 Member

@czolnowski thomas created issue at #6609 but you don't need to solve it now, let's stay focused on this small feature for this PR

@tsteur commented on November 7th 2014 Member

Hacks / adding dependencies shouldn't be justified just because it is a small feature. Then we'll always have this mess ;) And using Settings API should be easy to use

@mattab commented on November 7th 2014 Member

@tsteur of course I agree that we should find better solution directly, but here we have already done this before (see setting scheduled_reports_replyto_is_user_email_and_alias) so IMO it's acceptable for Marcin to make this PR and us to merge it. also of course you're welcome to put #6609 in Short term

@czolnowski commented on November 7th 2014 Contributor

@mattab: I have renamed this config parameter according to your guidelines.

@mattab commented on November 8th 2014 Member

Cheers! fyi it's not 'my' guideline but it's to be consistent with existing setting just above in the file

This Pull Request was closed on November 8th 2014
Powered by GitHub Issue Mirror