[Debug][DebugClassLoader] Match callable() type for parameters not defined in sub classes #29988
Conversation
2957e8b to
89c89c9
Compare
|
Actually the current positive lookbehind on space must be kept for BC. It is useful since we consider that a type can contain a "$". It was just not tested. I added more test cases. Especially one that check that defined parameters in sub classes actually doesn't trigger the deprecation. The only type that can not be matched with this implementation is |
|
fabbot patch should not be applied or it will test less cases with different spaces. |
89c89c9 to
0f94713
Compare
|
Something went wrong when merging this pull request. @fancyweb Can you push your code again please? |
|
Should also be rebased on 4.2, right? |
…meters not defined in sub classes (fancyweb) This PR was merged into the 4.3-dev branch. Discussion ---------- [Debug][DebugClassLoader] Match callable() type for parameters not defined in sub classes | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29969 | License | MIT | Doc PR | - Added test fixture for multi param, no param type and multi spaces as well. BTW, I didn't understand why there was a positive lookbehind on the current regex. Looks useless to me and tests passes without it. Commits ------- 89c89c9 [Debug][DebugClassLoader] Match callable() type for parameters not defined in sub classes
|
Just force repushed but it looks like the code was merged on master. @nicolas-grekas said this should be a new feature (so master branch) on the linked issue. |
|
It's been merge on master, right, so all is fine? |
|
Yes => 3659e32 |
Added test fixture for multi param, no param type and multi spaces as well.
BTW, I didn't understand why there was a positive lookbehind on the current regex. Looks useless to me and tests passes without it.