Skip to content

Conversation

@oliverklee
Copy link
Contributor

Note: The console will only be added if this project is the project root
(i.e., usually when developing the package), not if is a requirement of
the base-distribution package (or any other package).

@oliverklee oliverklee added this to the 4.0.0 ("phase 2") milestone Jul 27, 2017
@oliverklee oliverklee self-assigned this Jul 27, 2017
@oliverklee oliverklee requested a review from samtuke July 27, 2017 12:21
@oliverklee oliverklee mentioned this pull request Jul 27, 2017
14 tasks
@@ -0,0 +1,44 @@
<?php
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script appears to be a duplicate from the base-distribution package (minus unit tests) -- will it be removed from there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not that easy. Currently, the script is expected to be in the root package only (at least when it is called), i.e. in the base distributions for the application installation, and in rest-api when testing the rest-api without the base-distribution. It currently relies on this fact to determine the root package path.

I'll use a different approach instead: I'm currently working on a function for determining the application root path. I'll then move the Composer ScriptHandler to the core package and use that from base-distribution, rest-api and web-frontend.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hence marking this PR as blocked. (The base-distribution PR is not blocked, though.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good plan

@oliverklee
Copy link
Contributor Author

This PR now is using the corresponding core functions. I've also added automatic creation of the web/ folder (which just like bin/ is also for development purposes only).

Currently, this build failing locally for me in the Controller integration tests due to a bug in the core concerning the application root directory. I'll look into it.

Note: The console and the web directory will only be added if this project
is the project root (i.e., usually when developing the package), not if
is a requirement of the base-distribution package (or any other package).
@oliverklee oliverklee changed the title [FEATURE] Add the Symfony console [FEATURE] Add the Symfony console and the public web/ directory Jul 28, 2017
@oliverklee
Copy link
Contributor Author

This PR fixes the problem: phpList/core#144

@samtuke samtuke merged commit 4a69bcd into master Jul 28, 2017
@samtuke samtuke deleted the feature/console branch July 28, 2017 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants