flatten cookies for header #92
Conversation
|
Could you provide a unittest for this? |
|
@jaapio I created a test can you please recheck? |
|
I can. But I won't be able to approve your pr since I'm not a zf
maintainer.
Op do 8 dec. 2016 13:15 schreef tflori <notifications@github.com>:
… @jaapio <https://github.com/jaapio> I created a test can you please
recheck?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#92 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABAuUbKsKb5B47_XcFHq_bQpqw2BUH7qks5rF_T1gaJpZM4KiQ37>
.
|
|
@weierophinney it would be greate if you could review this pr. thanks in advance. |
| $nvPairs[] = $name . '=' . (($this->encodeValue) ? urlencode($value) : $value); | ||
| } | ||
|
|
||
| return implode('; ', $nvPairs); | ||
| } | ||
|
|
||
| protected function flattenCookies(&$result, $data, $prefix = null) |
There was a problem hiding this comment.
Why we need a recursive function here? Do we need to manage array of array values in cookies? I think we should simplify the implementation here, for instance just iterating on keys and values of $value, if it is an array.
There was a problem hiding this comment.
Btw, we discourage the usage of passing value by reference in function. A good use case for reference is performance reason, but this is not the case.
There was a problem hiding this comment.
For me the correct way seems to be what php is doing. So yes we need a recursive function. But you are right we can write it without passing by reference.
|
@ezimuel I modified the function not to use pass by reference. I don't know why coveralls shows a decreased coverage status - the function I added is tested. Nothing else changed. |
|
@tflori thanks for your contribution! I improved the unit test with a bi-dimensional cookie array. |
Fix issue #77