[REF] Remove unused parameters #20301
Conversation
|
(Standard links)
|
This takes out some parameters that are never passed in & stops returning things that are unused
dcb8873 to
c92325a
Compare
| // @todo stop passing $ids (membership and userId may be set by this point) | ||
| $membership = CRM_Member_BAO_Membership::create($memParams, $ids); | ||
|
|
||
| // not sure why this statement is here, seems quite odd :( - Lobo: 12/26/2010 | ||
| // related to: http://forum.civicrm.org/index.php/topic,11416.msg49072.html#msg49072 | ||
| $membership->find(TRUE); | ||
|
|
||
| return [$membership, $renewalMode, $dates]; | ||
| return $membership; |
There was a problem hiding this comment.
Grepped universe for references to legacyProcessMembership - nothing outside of civicrm-core using it.
@eileenmcnaughton Just to double-check, the legacyProcessMembership in CRM/Contribute/Form/Contribution/Confirm.php and CRM/Batch/Form/Entry.php are essentially independent, right? So it looks like this change to the Batch/Form/Entry.php has no interaction with the Confirm.php one.
If so, I'd say go ahead and merge.
There was a problem hiding this comment.
I believe we have unit testes that cover the Confirm.php stuff as well
|
Merging as per review from @totten and myself |
|
thanks @totten & @seamuslee001 |
|
It looks like converting it to protected would have made this easier & since you've done the review work to establish it it makes sense to to it now before that 'goes cold' - I also transferred hard-coding into the function |
Overview
[REF] Remove unused parameters
Before
We have duplicated this function so it is no longer shared. Many parts do not apply to the class it is now on
After
This takes out some parameters that are never passed in & stops returning things that are unused. Baby step
Technical Details
Comments
test cover in the EntryTest - I made minor cleanups to it for visibility