NFC cleanup in test class #25524
NFC cleanup in test class #25524
Conversation
|
(Standard links)
|
| @@ -518,7 +516,7 @@ public function testDupesByParams() { | |||
| * Locks in expected params | |||
| * | |||
| */ | |||
| public function hook_civicrm_findDuplicates($dedupeParams, &$dedupeResults, $contextParams) { | |||
| public function hook_civicrm_findDuplicates($dedupeParams, $dedupeResults, $contextParams) { | |||
There was a problem hiding this comment.
This doesn't match the dev docs, and the return at the bottom. Are the docs wrong?
Otherwise everything in the PR looks good.
There was a problem hiding this comment.
Actually I think the docs are right and the test is half-right. The signature was correct, but the return is wrong, it just doesn't come up because the hook implementation never changes the value.
There was a problem hiding this comment.
@demeritcowboy yeah - I normally remove the pass by ref in extension hooks too if they don't leverage it - just makes it all clearer
There was a problem hiding this comment.
@demeritcowboy I didn't want this tidy up to be blocking on the actual fix - #25527 - so I reverted it back & added php doc block to ignore it.
eileenmcnaughton
force-pushed
the
finder_test
branch
from
47b0a0e to
3e823c3
Compare
|
unrelated style-check fail which isn't really a style fail. I still think the |
No description provided.