Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

pass configured input filter plugin manager to generated input filter factories #77

Closed
wants to merge 1 commit into from

Conversation

abacaphiliac
Copy link

pass configured input filter plugin manager to generated input filter factories to allow for complex input filter nesting. this enables reuse of named input filter services within nested input filter contexts.

i added more logic in the factory than i had originally intended, in order to preserve backwards compatibility.

@Maks3w
Copy link
Member

Maks3w commented Oct 24, 2015

  1. You've added 3 new conditions but there's only 1 test. Should be at least 5.
  2. We are adopting SM v3 across all repositories and I'll wait to this task to be over before merge this

… factories to allow for complex input filter nesting. this enables reuse of named input filter services within nested input filter contexts.
@abacaphiliac
Copy link
Author

@Maks3w thanks for pointing out the lack of path coverage. i was able to reduce a bit of the complexity and i think my update now includes the line and path coverage you expect.

since it sounds like this must wait, would you like me to open a new pull request to develop instead of master?

do you know where i can find information on when SM v3 will be completed? i'm interested in reviewing the changes and contributing, if possible. do you anticipate that the new major version of this dependency will require a major version change for this package as well?

@Maks3w
Copy link
Member

Maks3w commented Oct 24, 2015

Target branch not affect to this PR to be on hold after the SM upgrade is done.

If you want you can forward that task proposing the SM adoption

Examples

zendframework/zend-serializer#2
zendframework/zend-cache#22
zendframework/zend-mvc#36

@weierophinney
Copy link
Member

#3, merged today, accomplishes this, and will release with a forthcoming 2.7.0.

@abacaphiliac
Copy link
Author

excellent! thanks @weierophinney : )

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

Successfully merging this pull request may close these issues.

3 participants