(REF) Remove constructors that do nothing #22544
Conversation
|
(Standard links)
|
|
@colemanw looks like Jenkins isn't buying it |
This pass-by-ref was causing `civilint` to crash and appears to be unnecessary and possibly harmful.
|
@eileenmcnaughton the |
| * @return \CRM_Financial_DAO_FinancialTrxn | ||
| */ | ||
|
|
||
| /** |
There was a problem hiding this comment.
this will make @pradpnayak's PR stale - should be an easy tweak to that PR but it might come back depending on how the merge conflict fixing goes...
There was a problem hiding this comment.
This PR is probably easier to rebase if you wanna merge that one first.
There was a problem hiding this comment.
@colemanw #22400 is currently failed tests - I pulled out #22546 as a partial that renames the class & adds in the temporary placeholder class that extends the new 'real' class and removed the constructor. Hopefully that passes & has the changes from both PRs that touch this file (without having an answer on the failing tests at this stage)
There was a problem hiding this comment.
@colemanw this got merged in the meantime - turns out the test error @pradpnayak was hitting is still present in the smallest version of the patch & I can't run the test myself to see why
Overview
Removes unused code
Technical Details
Contrary to what some of the code comments say, these functions do nothing and return nothing.
Comments
Still fighting the good fight begun in #22417