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

Support passing old method name into deprecatedFunctionWarning #17552

Merged

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jun 8, 2020

Overview

When I originally added this function it replaced a lot of adhoc, inconsistent log messages that were doing the same thing. We are now using this function extensively to indicate that things are deprecated but it does not always work because the calling method is not always the deprecated one. Eg. see #17456 for an example (there are already examples in core where we are doing similar but overriding the $newMethod text to include the name of the old method etc.

Before

Doesn't work well when caller is not the deprecated method.

After

Works well when caller is deprecated and when caller is not deprecated.

Technical Details

Add an optional second param $oldMethod

Comments

@eileenmcnaughton

@civibot
Copy link

civibot bot commented Jun 8, 2020

(Standard links)

@@ -1035,12 +1035,17 @@ public static function isAPIError($error, $type = CRM_Core_Error::FATAL_ERROR) {
*
* @param $newMethod
* description of new method (eg. "buildOptions() method in the appropriate BAO object").
* @param $oldMethod
Copy link
Contributor

Choose a reason for hiding this comment

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

@mattwire my only issue is this comment block - can we add the type (string) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Done.

@colemanw
Copy link
Member

colemanw commented Jun 8, 2020

The other thing is that the thing that's deprecated is not always a function. Sometimes it's a param.
Perhaps the message could be "$oldMethod is deprecated in favor of $newMethod."

@eileenmcnaughton eileenmcnaughton merged commit 49b2c76 into civicrm:master Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants