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

Extra deprecation for clarity on token function convertPseudoConstantsUsingMetadata #25511

Merged
merged 1 commit into from
Feb 21, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Extra deprecation for clarity on token function

Before

The only function this is called from is, itself, noisily deprecated. However, you have to do a search to find out this is deprecated

image

After

Deprecation is more obvious, not much change for anything that hits this path as there is already a warning

Technical Details

Comments

@civibot
Copy link

civibot bot commented Feb 6, 2023

(Standard links)

@civibot civibot bot added the master label Feb 6, 2023
@totten
Copy link
Member

totten commented Feb 21, 2023

+1 for the @deprecated flag. It discourages new calls and helps to show the progression of the phase-out.

But for the logging... you're saying there's already a logged notice. So instead of generating 1 log-notice, it generates 2 notices? (Or, actually, n-many -- because the call from getContactTokenReplacement() has a loop around convertPseudoConstantsUsingMetadata().) And the person who reads the notice shouldn't/can't do anything about convertPseudoConstantsUsingMetadata(). It's just generally amplifying the noise?

@eileenmcnaughton
Copy link
Contributor Author

@totten yeah it may amplify the noise - but I doubt anyone will notice since they are already getting noise

The bigger thing is that when looking at the function it is easy to see that it is progressing well on it's way out. We spend a lot of time grepping to see if functions are called & my goal is once one has decided there is no undeprecated path to call a function no-one else ever has to do that grep again

@eileenmcnaughton
Copy link
Contributor Author

& yeah having both makes a big difference in that calculation - cos you still have to do the grep next time if only the comment is present - eg. I want to know what undeprecated paths call CRM_Contact_BAO_Contact::exportableFields & I really need to find out when deciding that if they are quietly or noisily deprecated so there is that extra grep

@totten totten merged commit 34a2e86 into civicrm:master Feb 21, 2023
@totten totten deleted the dep_token6 branch February 21, 2023 05:08
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