silverstripe / cwp-installer

CWP project template
BSD 3-Clause "New" or "Revised" License
0 stars 5 forks source link

API Respect SS_DATABASE_CHOOSE_NAME #1

Closed robbieaverill closed 6 years ago

robbieaverill commented 6 years ago

Migrated from GitLab

cc @tractorcow

I'm sick of all of my local projects using the same SS_cwp database when a better alternative is available.

This will make cwp developing better across multiple projects. :)

// find the database name from the environment file
if(defined('SS_DATABASE_NAME') && SS_DATABASE_NAME) {
    $database = SS_DATABASE_NAME;
- } else {
+ } elseif(!defined('SS_DATABASE_CHOOSE_NAME') || !SS_DATABASE_CHOOSE_NAME) {
    $database = 'SS_cwp';
}
tractorcow commented 6 years ago

Ah yes please! That would be great. :)

NightJar commented 6 years ago

With SS4 the configuration is both always from the environment, and always before processing _config, so this is no longer an issue. There is no second chance to set a database name via the pseudo-environment (SilverStripe\Core\Environment) if one wasn't specified; Bootstrapping will have already failed with an error.

robbieaverill commented 6 years ago

This config is removed in #10

robbieaverill commented 6 years ago

Reopened for CWP 1.x resolution via #11. @NightJar should we merge up the branches to get the change back into the CWP 2.x version and switch it for env vars? I noticed it was removed in #10 for CWP 2.x

NightJar commented 6 years ago

Merging up would still result in the same end result; the _config.php would be removed as it is completely unnecessary in the 2.x line. I don't think it's worth the effort, particularly for such a trivial change (which would then just be discarded again).

robbieaverill commented 6 years ago

Ok, good justification

dhensby commented 6 years ago

should this be closed?

NightJar commented 6 years ago

Wait for #11 since it was originally (and still is) labelled affects/v3

robbieaverill commented 6 years ago

Fixed in #11