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

Add $modx->deprecated method to log usage of deprecated methods #14127

Merged
merged 4 commits into from
Nov 5, 2018

Conversation

Mark-H
Copy link
Collaborator

@Mark-H Mark-H commented Nov 3, 2018

What does it do?

This PR adds a new method to the modX object, deprecated, which is to be used to log usage of deprecated methods across the core. Each unique message (deprecated method + caller) is only logged once per request. A setting is added that allows disabling the logging.

I've also done a search for @deprecated docblocks, and added appropriate messages in various locations. Some code was deprecated way back in 2.1, so I look forward to seeing that removed in 3.0.

Why is it needed?

As we get ready to cleanup some code with 3.0, it's important users can be sure their site can be safely upgraded. Old or custom extras may use functionality that is no longer supported and will be removed in 3.0, which will require manual intervention.

The deprecated method will allow a site to function normally with the deprecated methods still available in 2.7, but thanks to the logging the site owner will have more information on what should be taken care of first.

Related issue(s)/PR(s)

#13199, which mentions some deprecated methods for 3.0.

This is to be used as a standardised way to mark functionality as deprecated, by calling it from within the deprecated function with at least the version it was marked as deprecated in. Additional information like the calling function is automatically added, to ease identifying code that needs to be updated.

If the deprecated function cannot be automatically determined properly (e.g. the deprecated function is a snippet, which would show up as "include" in "modScript" being deprecated, or if a deprecation pertains about the usage of a full object) the third param can be provided to manually name the deprecated function.
@Mark-H Mark-H added this to the v2.7.0 milestone Nov 3, 2018
@Mark-H Mark-H requested a review from opengeek as a code owner November 3, 2018 21:26
Jako
Jako previously requested changes Nov 4, 2018
Copy link
Collaborator

@Jako Jako left a comment

Choose a reason for hiding this comment

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

I don't have a clue, why this line is commented out.

core/model/modx/modrequest.class.php Outdated Show resolved Hide resolved
…t we cannot yet deprecate

It's still in use in the core, so would cause an entry to the log on each manager request, which needs to be resolved first in a spearate PR that handles the removal of modAction.
Looking forward to the day we can use short syntax all over the core. :D
@Mark-H Mark-H added area-core pr/review-needed Pull request requires review and testing. and removed state/in-progress labels Nov 5, 2018
Copy link
Member

@opengeek opengeek left a comment

Choose a reason for hiding this comment

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

This may not be the ideal solution, but it will work for now and provides an abstraction for better solutions moving forward.

@opengeek opengeek dismissed Jako’s stale review November 5, 2018 16:49

Changes in question were taken out.

@opengeek opengeek merged commit 228babe into modxcms:2.x Nov 5, 2018
@Mark-H Mark-H deleted the 2.7-deprecation-notices branch April 25, 2019 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core pr/review-needed Pull request requires review and testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants