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] Minor code cleanup #17225

Merged
merged 1 commit into from
May 4, 2020
Merged

[REF] Minor code cleanup #17225

merged 1 commit into from
May 4, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Do not pass params as reference as unchanged

Before

function takes &$params,

After

function takes $params, other minor cleanups

Technical Details

Comments

@civibot
Copy link

civibot bot commented May 3, 2020

(Standard links)

@civibot civibot bot added the master label May 3, 2020
CRM_Utils_Array::value('name', $v, '') == 'image_URL' ||
CRM_Utils_Array::value('field_type', $v) == 'Formatting'
) {
$v['data_type'] ?? '' === 'File' || $v['name'] ?? '' === 'image_URL' || $v['field_type'] === 'Formatting') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I first looked at this, I thought the precidence of the ?? operators and the === operators might be ambiguous and we should add parentheses.

On second look, I see that the array data is coming from CRM_Core_BAO_UFGroup::getFields which runs every item through CRM_Core_BAO_UFGroup::formatUFField which ensures that data_type, name, and field_type keys are always present. So I don't think the coalesce is needed and this is just another historical artifact of our developers' love-affair with CRM_Utils_Array::value() using it places where it isn't needed, "just because".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh - let's remove!

Do not pass params as reference as unchanged
CRM_Utils_Array::value('name', $v, '') == 'image_URL' ||
CRM_Utils_Array::value('field_type', $v) == 'Formatting'
) {
$v['data_type'] === 'File' || $v['name'] === 'image_URL' || $v['field_type'] === 'Formatting') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colemanw tada

@colemanw colemanw merged commit 3561971 into civicrm:master May 4, 2020
@colemanw colemanw deleted the cont branch May 4, 2020 01:13
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.

2 participants