Skip to content
This repository has been archived by the owner on Aug 28, 2018. It is now read-only.

Add support for application kernels which are not named AppKernel #48

Merged
merged 2 commits into from
Oct 27, 2014

Conversation

dsbaars
Copy link
Collaborator

@dsbaars dsbaars commented Oct 11, 2014

Cleaner solution than #33, because it checks on implementation of the KernelInterface.

I also adjusted the tests for this and fixed some MongoDB tests which assumed the return of a cursor, when it was an array.
Also, I improved adherence to the PHP coding standard for some lines of code.

…fixed some coding-style errors and MongoDB tests in the progress
@dsbaars dsbaars changed the title Add support for application kernels which are not named AppKerel Add support for application kernels which are not named AppKernel Oct 11, 2014
@@ -14,10 +14,29 @@ protected function configure()
$this
Copy link
Owner

Choose a reason for hiding this comment

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

Only changing formatting here? Any reason?

@khepin
Copy link
Owner

khepin commented Oct 22, 2014

Why all the format changes? It makes it really difficult to find out where are the actual changes to the code and review them.

Also it seems you didn't add a test for your use case.

@dsbaars
Copy link
Collaborator Author

dsbaars commented Oct 25, 2014

The format changes makes this bundle adhere to the PHP coding standards (http://symfony.com/doc/2.3/contributing/code/standards.html)

Furthermore, the testing code only needed to be modified. See: https://github.com/dsbaars/KhepinYamlFixturesBundle/blob/a13260ccdaada0aa461b7cf2fe8e44b05181d61d/tests/Khepin/Tests/YamlFixtureTest.php#L26

khepin added a commit that referenced this pull request Oct 27, 2014
Add support for application kernels which are not named AppKernel
@khepin khepin merged commit a4ffd81 into khepin:master Oct 27, 2014
@khepin
Copy link
Owner

khepin commented Oct 27, 2014

Alright, thanks for clarification. Sorry for the delay, I am moving to a new country, lots of things to do. I'm adding you to the repo. You'll be able to publish a version if needed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants