-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix #5671 - console routing not correct #5720
Conversation
@mtymek I'll take care of merge conflicts when I merge to develop for 2.3.0. |
Fix #5671 - console routing not correct Conflicts: library/Zend/Mvc/Router/Console/Simple.php
Merged to develop for release with 2.3.0. |
@weierophinney I know, that's exactly why I wanted to help with merge :) |
'mandatory-literal-camel-case' => array( | ||
'FooBar', | ||
array('FooBar'), | ||
array(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this test case.
Why is the expected result an empty array()
? If you're defining a mandatory literal parameter, then the result must be array('FooBar' => null)
- see previous test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a block of literal parameters' test cases: https://github.com/mtymek/zf2/blob/9b97b51f0c1b00934999f81b6a7f4a4d217a7aa9/tests/ZendTest/Mvc/Router/Console/SimpleTest.php#L303-L362
@Thinkscape I briefly looked at the matching code, and most likely it is just the test - doesn't fail where it should. I will fix it in coming days. |
@mtymek Please handle it before 2.3.0 |
As I wrote, it was just an issue with unit test: #5943. |
…x/5671-console-routing Fix zendframework/zendframework#5671 - console routing not correct Conflicts: library/Zend/Mvc/Router/Console/Simple.php
This PR fixes problems in console route matching, reported in #5671:
ping @Thinkscape
BTW, it doesn't merge cleanly with develop branch. What is a workflow for PRs here? Would it work if I did cherry-pick this, cleanup, commit, and create new PR against develop?