silverstripe / recipe-core

Silverstripe recipe for a framework-only minimal install
BSD 3-Clause "New" or "Revised" License
4 stars 13 forks source link

Requires silverstripe/versioned? #31

Open robingram opened 6 years ago

robingram commented 6 years ago

Should this recipe require silverstripe/versioned? On a clean install I had to add it myself via composer to get /dev/build to run.

robbieaverill commented 6 years ago

There were a couple of PRs recently to remotely some coupling on versioned in other modules. I think the objective should be to keep it separate and fix the points that are broken, rather than add it to the dependencies

maxime-rainville commented 6 years ago

I'm with Robbie. If Core doesn't build without versioned that problem should be addressed by decoupling versioned from core.

We don't currently have unit test on recipe-core. We might to add some basic ones to confirm, core can build and be functional by itself.

robbieaverill commented 6 years ago

We do have Travis builds now, we could add some with and without versioned. To be fair, if it’s not a dependency then Travis should mostly be running tests without versioned as well

maxime-rainville commented 6 years ago

I manage to get recipe-core to build by itself without issue.

My composer file looked like this:

{
    "name": "maxime/recipe-core-project",
    "require": {
        "silverstripe/recipe-core": "^4.2"
    },
    "authors": [
        {
            "name": "Maxime Rainville"
        }
    ],
    "require-dev": {
        "phpunit/phpunit": "^5.7"
    },
    "extra": {
        "project-files-installed": [
            "app/.htaccess",
            "app/_config.php",
            "app/_config/mysite.yml"
        ],
        "public-files-installed": [
            ".htaccess",
            "index.php",
            "install-frameworkmissing.html",
            "install.php",
            "web.config"
        ]
    }
}

Here's the output I got

image

The unit tests for framework won't pass if versioned is not installed, but that's expected.

Probably still worth adding some basic unit test to recipe core to make sure the thing build.