[2.1] Update library/Zend/View/Helper/HeadScript.php #2208

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

superhero commented Aug 20, 2012

What this does:
This fix would result in that the 3rd param is the conditional string

Reason:
I would like to see the same pattern used for this method as for HeadLink::_call (eg. appendStylesheet, offsetSetStylesheet, prependStylesheet and setStylesheet)

Update library/Zend/View/Helper/HeadScript.php

What this does:
This fix would result in that the 3rd param is the conditional string

Reason:
I would like to see the same pattern used for this method as for HeadLink::_call (eg. appendStylesheet, offsetSetStylesheet, prependStylesheet and setStylesheet)

This pull request fails (merged 4b1fe75 into 5dd58b8).

Owner

weierophinney commented Aug 21, 2012

@erik-landvall Please see the failure report linked by @travisbot -- this introduces a regression, as you're changing argument order by adding the new argument between existing arguments. The new argument should be an optional final argument to the pseudo-methods.

Also, please create test cases.

@ghost ghost assigned weierophinney Aug 21, 2012

Contributor

superhero commented Aug 21, 2012

For this method to be able to follow the same pattern as the ones in HeadLink I would argue that the arguments should be in this order instead. I open a new pull request in a more correct fashion later.

@superhero superhero closed this Aug 21, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment