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] Stop returning unused variables #22401

Merged
merged 1 commit into from
Jan 9, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jan 7, 2022

Overview

Only 2 code places call this function (one an unsupported call outside core but in universe & the other in Parser_Contact)
and only the latter uses ANY of the return variables - this fixes the function to stop
returning those that are never used - this stops them from returning them

Before

Only the first & third return values are used from the 2 places that call the function
Core usage in Parser_Contact

image

Unsupported extension usage

image

After

Only the used values are returned

Technical Details

@seamuslee001 this won't break the jma grantapplications extension as you can see from the above screenshot (and none of my patches do as yet) - but the functions was deprecated in 4.6 so really the extension should call the api.... is it still used?

Comments

@civibot
Copy link

civibot bot commented Jan 7, 2022

(Standard links)

@@ -146,82 +146,6 @@ public function testRelationshipTypeAddStudentSponcor() {
$this->relationshipTypeDelete($result->id);
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove tests that cover unused functionality

Only 2 code places call this function (one an unsupported call outside core but in universe & the other in Parser_Contact)
and only the latter uses ANY of the return variables - this fixes the function to stop
returning those that are never used  - this stops them from returning them
@eileenmcnaughton
Copy link
Contributor Author

@colemanw can we merge this one?

@colemanw colemanw merged commit d85dcdd into civicrm:master Jan 9, 2022
@colemanw colemanw deleted the legit branch January 9, 2022 04:37
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