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

TaskLib, PSR-3 logger, and interim DI container. #283

Closed
wants to merge 37 commits into from

Conversation

greg-1-anderson
Copy link
Member

Split off TaskLib from Robo\Tasks. Add a Dependency Injection container, and a PSR-3 logger based on Consolidation\Log.

Continuation of #273. Squashed and rebased with master.

…er, and a PSR-3 logger based on Consolidation\Log.
@greg-1-anderson
Copy link
Member Author

Regarding which DI Container to use, see How to Configure Monolog to Display Console Messages as an example. We want to start doing some bridging with existing code based on Symfony Console, so we'll want to make it easy to do things like $logger = $this->getContainer()->get('logger'); (or similar, for other services). I've mentioned this desire for interoperability with Symfony DI in Drush for Drupal 8, but wanted to put in this link as another example.

I further improved the decoupling of the code in #284; although there are still a couple of backwards-compatibility-breaking changes needed to fully separate tasks and all task implementations from the DI container and Config class, that PR and this one gets Robo really close. If the changes here & there are okay, next steps are largely dependent on what we want to do vis-a-vis versioning and backwards compatibility.

@DavertMik
Copy link
Member

looks like this PR can be closed now, right?

@greg-1-anderson
Copy link
Member Author

This code should have been merged into master when you merged task-assembly, since I branched that PR off of this one. I may have rebased in the middle, confusing GitHub. I'll confirm the state of master and close here.

@greg-1-anderson
Copy link
Member Author

This has been merged.

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.

2 participants