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

Added "helper" functionality #9

Closed
wants to merge 2 commits into from
Closed

Added "helper" functionality #9

wants to merge 2 commits into from

Conversation

dantleech
Copy link
Member

This PR enables us to do

$this->helper('DependencyInjection')

to retrieve special helper classes. It is inspired by the IcBaseTestBundle.

Helper classes can implements ContainerAwareInterface to have access
to the container.

Have included a helper for DependecyInjection to help with schema validation.

This PR enables us to do

````
$this->helper('DependencyInjection')
````

to retrieve special helper classes.

Helper classes can implements `ContainerAwareInterface` to have access
to the container.

Have included a helper for DependecyInjection
@@ -25,6 +26,31 @@ public function db($type)
return $this->getDbManager($type);
}

public function helper($name)
{
$name = ucfirst($name).'Helper';
Copy link
Member

Choose a reason for hiding this comment

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

imo, it's cleaner to put this in the sprintf

@wouterj
Copy link
Member

wouterj commented Jul 31, 2013

Not sure if I like it that we create a new instance of the helper each time. This is inconsistent with Console Helpers (which are quite related to these helpers)

@dantleech
Copy link
Member Author

Yeah, I can cache that. Might cause problems if helpers are stateful though.

@wouterj
Copy link
Member

wouterj commented Jul 31, 2013

And that's the reason why almost nothing of the console component is statefull

@dbu
Copy link
Member

dbu commented Aug 1, 2013

looks ok. i wonder however if it does not mean its time to start using the IcTestBundle thingy, if we need to add code from their bundle here...

@dantleech
Copy link
Member Author

Well, the IcTestBundle doesn't support XML schema validation. I just don't want to add a dependency that doesn't "fit"and that we have no control over. The code we add is very simple in anycase, and IcTest doesn't support PhpcrOdm.

@dbu
Copy link
Member

dbu commented Aug 4, 2013

we could try to contribute that to IcTestBundle too :-)

but anyways, i am ok if you want to merge this, go for it.

@dbu
Copy link
Member

dbu commented Nov 4, 2013

@dantleech what is the state of this? should we update the branch and merge, or drop it?

@wouterj
Copy link
Member

wouterj commented Nov 4, 2013

helper functionality +1
dependency helper -1 (I prefer matthiasnoback/SymfoyConfigTest instead)

@wouterj wouterj mentioned this pull request Dec 21, 2013
@wouterj
Copy link
Member

wouterj commented Dec 28, 2013

Closing as it's replaced by #33 and we don't need the helper functionality at the moment (and I also don't like the concept of helpers).

@wouterj wouterj closed this Dec 28, 2013
@wouterj wouterj deleted the helpers branch December 28, 2013 11:09
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