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

Preserve the fluent interface #6398

Closed
wants to merge 1 commit into from
Closed

Preserve the fluent interface #6398

wants to merge 1 commit into from

Conversation

Darhazer
Copy link
Contributor

Not returning $this from the setter breaks the fluent interface.

@jmleroux
Copy link
Contributor

Fluent interface is not always the preferred way.
@Ocramius wrote an article on the subject.

@Darhazer
Copy link
Contributor Author

Sure, but the other setter in the class (setMaxNestedForwards) returns $this. So it's kinda confusing

@jmleroux
Copy link
Contributor

Does it make sense to use it in others setters ?
But removing them coul be a BC break.

@Ocramius ?

@Ocramius
Copy link
Member

Consistency before nitpicking in my opinion, so I'm fine with this change.
Fluent should probably be stripped wherever possible in a major release in my opinion.

Ocramius
Ocramius reviewed Jun 20, 2014
View reviewed changes
library/Zend/Mvc/Controller/Plugin/Forward.php
@@ -94,11 +94,12 @@ public function getListenersToDetach()
* Set information on listeners that need to be detached before dispatching.
*
* @param array $listeners Listener information; see getListenersToDetach() for details on format.
* @return void
* @return Forward
Copy link
Member

Choose a reason for hiding this comment

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

self

@Ocramius
Copy link
Member

@Darhazer can you please add a test case?

@Ocramius Ocramius self-assigned this Nov 17, 2014
@Ocramius Ocramius added this to the 2.3.4 milestone Nov 17, 2014
Ocramius added a commit that referenced this pull request Nov 17, 2014
@Ocramius
#6398 - minor CS fixes (spacing, return type)
b1f4599
Ocramius added a commit that referenced this pull request Nov 17, 2014
@Ocramius
#6398 - minor CS fixes (spacing, return type)
a8b90d1
Ocramius added a commit that referenced this pull request Nov 17, 2014
@Ocramius
#6398 - type fix (listenersToDetach may be null)
d4cdbc6
Ocramius added a commit that referenced this pull request Nov 17, 2014
@Ocramius
#6398 - test case to enforce fluent interface on `Forward#setListener…
9136794 
…sToDetach()`
Ocramius added a commit that referenced this pull request Nov 17, 2014
@Ocramius
#6398 - Adding missing test-case properties and type-hints
1bd49a1
Ocramius added a commit that referenced this pull request Nov 17, 2014
@Ocramius
Merge branch 'hotfix/#6398-forward-controller-plugin-fluent-interface…
d24d5ca 
…-consistency' into develop

Close #6398
Forward port #6398
@Ocramius Ocramius closed this in 4ab7cda Nov 17, 2014
@Ocramius
Copy link
Member

@Darhazer manually rebased/merged, thanks!

master: 4ab7cda
develop: d24d5ca

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@Ocramius Ocramius

Labels
enhancement Mvc
Projects
None yet
Milestone
2.3.4
Development

Successfully merging this pull request may close these issues.
4 participants
@Darhazer @jmleroux @Ocramius @ezimuel