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

Bare minimum composer installation #27

Open
mlocati opened this issue May 8, 2019 · 9 comments
Open

Bare minimum composer installation #27

mlocati opened this issue May 8, 2019 · 9 comments

Comments

@mlocati
Copy link
Collaborator

mlocati commented May 8, 2019

By running composer create-project concrete5/composer, we end up with a setup that includes PHPUnit, webmix, custom db settings, ...
That could be nice for developers (provided that they want all that stuff), but end users may just want a running concrete5 setup...

So, we should be able to create a concrete5 instance without all that developer-oriented stuff.

What about having 2 composer projects?

  • concrete5/composer with only the bare minimum stuff required to install concrete5
  • concrete5/composer-dev with additional stuff targeting development
@KorvinSzanto
Copy link
Member

I think splitting these out makes sense, but I'm reluctant because I find these things to be immensely important and helpful when starting a new project. I really appreciate how Laravel gets you started with a working test suite and encourages writing tests for projects.

A few more suggestions I saw / thought of:

  • Have some tooling in the project that allows you to enable / disable parts of the skeleton
    I think this is a good idea, but I wonder what the best way to do it would be. I've really wanted to build a tool that will generate common constructs during development, I don't know that webpack / mix and phpunit is a great target for that though. I do think having tooling that initializes some of the settings and gets rid of the default namespacing would be ideal.
  • Consolidate this and the package skeleton so that their naming is similar and not confusing
    Sample_composer_package is an awful name, let's name it something different that meshes with the skeleton package and makes it more obvious that it's intended to be a skeleton
  • Consider using different terms for these.
    "boilerplate" comes to mind, but I honestly don't know how that's different from "Skeleton". Maybe there's a way to fit "project template" in there somehow.

@eskema
Copy link

eskema commented May 8, 2019

My opinion on the subject: I would like to be able to install c5 with composer as this is the new preferred way, but don't really use any of the developer features (I know maybe I should start using it, but I wouldn't know where to begin right now), so an option to install just the bare minimum would be a good option for my use case.

@ahukkanen
Copy link

I would also advocate for a more fully fledged defaults with all the "best practices" type of stuff baked in. It's much easier to delete this stuff than add it afterwards. I would even take the PHPUnit tests one step further to allow easier start with the concrete5 database tests. I ended up recently spending half a day setting up a basic concrete5 database test set for a project.

One thing I also don't like in the current setup is the ConcreteComposer namespace but that may be completely another discussion. I don't think those classes should have either "Concrete" or "Composer" in their name. The old Application\Src was much better in my opinion, although I understand why you might want to get rid of the Src in the namespace.

@mlocati
Copy link
Collaborator Author

mlocati commented May 10, 2019

I would also advocate for a more fully fledged defaults with all the "best practices" type of stuff baked in.

We have 2 different usages for composer-based concrete5 installation: developers and end-users.

For developers I agree that we should have a fully-featured starting point.
For end users, we should have a clean installation, with the minimum required contents to have concrete5 up and running.

The old Application\Src was much better in my opinion

That namespace points to PHP files in the public webroot. This ConcreteComposer sample namespace points to PHP files outside the webroot. And you can name it whatever you like, provided that it's listed in the autoload section of the composer.json file.

@ahukkanen
Copy link

That namespace points to PHP files in the public webroot. This ConcreteComposer sample namespace points to PHP files outside the webroot. And you can name it whatever you like, provided that it's listed in the autoload section of the composer.json file.

I am aware of both of this stuff but I understand the current best practices suggest towards keeping more and more stuff outside of the webroot (as IMO it should've been from day 0). I know I can change the namespace to whatever I want but we are talking here about the defaults. I haven't come up with a better namespace, however.

@katalysis
Copy link

I'm considering switching to Composer install for every day site builds (not really 'development') to speed up deployment and to take files out of the webroot etc. which I understand to be best practice.

If I try this now using the instructions provided on the concrete5.org/download page I get errors related to phpunit php requirements (my site is set using 7.2.1.9 but for some reason Composer thinks it's 7.1.30 as per @eskema's issue.)

It's taken me best part of an hour to wade through Slack and find my way here and I still don't have a working Composer based install.

A simpler Composer install for end users can only help wider adoption of concrete5.

@KorvinSzanto
Copy link
Member

I've fixed the main issue here which was that we required versions of PHPUnit and mockery that didn't support PHP 5.5.9 and I've released version 1.1.8.

I'm going to leave this issue open since we're also discussing the possibility of making this repository have nothing but the bare basics rather than boiler plate phpunit and asset building and all that. I'm not quite ready to do that yet but it's worth continuing discussion.

@SuperSchramm
Copy link

As an end-user, I would agree that you might want to go with two forks. I appear to be stuck trying to solve this issue:

Problem 1
    - Root composer.json requires concrete5/core dev-develop -> satisfiable by concrete5/core[dev-develop].
    - concrete5/core[dev-develop, 8.x-dev] require tedivm/stash 0.17.* -> found tedivm/stash[v0.17.0, v0.17.1] but the package is fixed to v0.16.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.

@KorvinSzanto
Copy link
Member

@SuperSchramm That error is complaining that your lockfile has an incompatible version of tedivm/stash. You can get past that with composer update concrete5/core tedivm/stash or by using --with-all-dependencies like the error suggests

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

No branches or pull requests

6 participants