Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move code files into the src/ folder. Fixes #6098 #6266

Merged
merged 4 commits into from
Nov 1, 2016
Merged

Move code files into the src/ folder. Fixes #6098 #6266

merged 4 commits into from
Nov 1, 2016

Conversation

sminnee
Copy link
Member

@sminnee sminnee commented Nov 1, 2016

No description provided.

This updates framework to be more in keeping with PHP conventions.
@tractorcow
Copy link
Contributor

tractorcow commented Nov 1, 2016

Bootstrapping Core.php and messing around with the path seems a pain. Maybe make it a proper class with a single static method init, and rely on class autoloading?

include_path shouldn't be necessary for anything other than legacy libraries pre-psr-4/0 really.

@tractorcow
Copy link
Contributor

tractorcow commented Nov 1, 2016

You could write it such a way that legacy code still works.

<?php

class Core {
    protected static $init = false;
    public static function init() {
        if (static::$init) {
            return;
        }
        static::$init = true;
        // init everything
    }
}
Core::init();

And now everywhere you had:

require_once $frameworkPath . '/Core/Core.php`;

You can now just

use SilverStripe\Framework\Core\Core;
Core::init();

sminnee pushed a commit to sminnee/silverstripe-behat-extension that referenced this pull request Nov 1, 2016
This lets us change the location of Core.php as long as we also
manipulate the include path.

This is necessary for silverstripe/silverstripe-framework#6266
@sminnee
Copy link
Member Author

sminnee commented Nov 1, 2016

I've gone with include_path because although I think that moving towards the use of classes is a good idea, I'd prefer to do that as part of a more considered refactor of the Core.php code. It's a bit soupy, and I'd prefer not to simply pour the soup into a class.

Sam Minnee and others added 3 commits November 1, 2016 16:35
Separated from the file renames for clarity.
FIX: Ensure that “npm run build” failures report an error code.
@tractorcow tractorcow merged commit c056362 into silverstripe:master Nov 1, 2016
@tractorcow tractorcow deleted the src-folder branch November 1, 2016 03:59
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.

None yet

2 participants