Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REF] simplify parameter, use preferred strict #19597

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Minor cleanup

Before

$b = $x;
$c = $x['values'];

After

$c = $b['values'];

Technical Details

In addition strict comparison is used for the in_array function - better coding practice but effectively the same here

Comments

@civibot
Copy link

civibot bot commented Feb 15, 2021

(Standard links)

@mattwire
Copy link
Contributor

@eileenmcnaughton The description doesn't match the code here!

@eileenmcnaughton
Copy link
Contributor Author

@mattwire - does it need a comma? - it simplifies the described parameter (although I have used shorter variables to illustrate the simplified pattern) & uses the preferred strict mode for comparisons

@mattwire
Copy link
Contributor

Actually I was thinking you were trying to update this bit of code https://github.com/civicrm/civicrm-core/blob/master/Civi/Token/TokenRow.php#L149 :-)

@eileenmcnaughton
Copy link
Contributor Author

@mattwire nope - the description just covers these changes

https://github.com/civicrm/civicrm-core/pull/19597/files#diff-c250584f6056eea3cae87f5cbd02b21dacc6bd31b4b72b697d9831d1f43c2396R99-R103

But I shortened it in the descriptor to be the concept not the exact same thing you would get if you read the diff

@colemanw
Copy link
Member

Man that in_array() bug is a real pain. I wish the default was strict, but it's good you're adding it now.

@colemanw colemanw merged commit d6a4e40 into civicrm:master Feb 19, 2021
@colemanw colemanw deleted the nfc branch February 19, 2021 20:54
@eileenmcnaughton
Copy link
Contributor Author

Yeah - it's only a theoretical issue in most cases - but it's best coding practice to use strict

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants