Skip to content

[WIP] Refactoring to use container #114

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

Merged
merged 1 commit into from
Dec 18, 2014
Merged

[WIP] Refactoring to use container #114

merged 1 commit into from
Dec 18, 2014

Conversation

dantleech
Copy link
Member

This PR introduces dependency injection, fixing #35.and tidying up the mess of helpers.

Todo:

  • Behat tests should pass
  • PHPUnit tests should pass
  • PHPSpec tests should pass
  • Test embedded application -- write tests for it
  • QusetionHelper is not a drop-in-replacement for DialogHelper. Either force minimum Symfony version to be 2.6 or add a fallback.
  • Run code fixer

use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class Container extends ContainerBuilder
Copy link
Member Author

Choose a reason for hiding this comment

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

Decided to use the Symfony DI Container instead of Pimple/whatever as I guess its more likely to be already available when run in embedded mode.

@dantleech dantleech changed the title Refactoring to use container [WIP] Refactoring to use container Dec 8, 2014
@dantleech dantleech modified the milestone: v1.0 Dec 8, 2014
@dbu
Copy link
Member

dbu commented Dec 9, 2014

looks good!
regarding requiring symfony 2.6: we could then no longer require this in the doctrine bundle i think. rather have graceful degradation, like less helpful interaction or something. doing the whole work double seems wrong too, however.

@dantleech
Copy link
Member Author

Well it should be possible and not too hard to add a fallback -- but saying that this isn't required by the bundle, its a suggests.. but as long as its not too hard it should be fine.

@dantleech
Copy link
Member Author

Have removed the EmbeddedApplication, instead the MODE should be passed to the Container, and the container passed to the ShellApplication.. hmm. maybe that doesn't work as the EmbeddedApplication does need to auto-exit in command mode.

Maybe EmbeddedApplication should instantiate its own Container after all.

@dantleech dantleech force-pushed the dicontainer branch 3 times, most recently from b9b47cd to 3971683 Compare December 13, 2014 11:35
@dantleech
Copy link
Member Author

  • ->askConfigmration on QuestionHelper does not exist
  • Make BC with 2.5 including adding dummy phpcr helper for existing DoctrinePHPCRBundle integration.

@@ -1,13 +1,18 @@
language: php

env:
- BEHAT_SUITE=standalone
- BEHAT_SUITE=embedded
Copy link
Member Author

Choose a reason for hiding this comment

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

Now testing with both modes of operation

- Upgraded to behat 3.0
- Use DI container for everything except commands
- Separate test suite for embedded mode
- CS fixes
dantleech added a commit that referenced this pull request Dec 18, 2014
[WIP] Refactoring to use container
@dantleech dantleech merged commit ff2b15d into master Dec 18, 2014
@dbu dbu deleted the dicontainer branch August 28, 2018 05:44
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.

3 participants