[REF] Follow up cleanup #17788
Merged
[REF] Follow up cleanup #17788
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This removes an IF block that relies on the always-true parameter I'm pretty sure it just wasn't done last round as all the white space created a lot of noise
|
(Standard links)
|
|
test this please |
| @@ -87,7 +87,7 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { | |||
| /** | |||
| * Details of the subscription (recurring contribution) to be altered. | |||
| * | |||
| * @var array | |||
| * @var \CRM_Core_DAO | |||
| */ | |||
| protected $subscriptionDetails = []; | |||
There was a problem hiding this comment.
It does seem to be a DAO, so now this initialization is weird.
Otherwise agree with everything in the PR and did a quick r-run seems ok.
There was a problem hiding this comment.
@demeritcowboy ouch - but TBH I'd rather punt on that as the change is right & I'm not touching this form other than comments
|
merging as per @demeritcowboy 's review |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This removes an IF block that relies on the always-true parameter
I'm pretty sure it just wasn't done last round as all the white space created a lot of noise
Before
If -> else exists but the condition for the IF is always true because this is the only place $cancelSubscription is referred to
https://github.com/civicrm/civicrm-core/pull/17788/files#diff-30a8e5b11544ab10573dea3f36cb9db2L202
After
Block removed
Technical Details
This is best viewed with the w=1 parameter - https://github.com/civicrm/civicrm-core/pull/17788/files?w=1 as it is otherwise hard to read. There are small additional cleanups:
Comments