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

Improve deprecated notice for "Old modTemplateVar getRender input" #14162

Merged
merged 2 commits into from Jan 8, 2019

Conversation

@Mark-H
Copy link
Collaborator

Mark-H commented Nov 28, 2018

What does it do?

Aside from changing the wording and adding context to make it a bit clearer what's the problem, this also moves the deprecated notice into the conditional.

Before, the message would get logged a lot for each first TV of a certain type due to the way the foreach handles registered render methods. Now it only gets logged when a deprecated flat file is used.

Why is it needed?

Make sure only relevant deprecated notices get logged and with appropriate context.

Related issue(s)/PR(s)

#14136

Improve deprecated notice for "Old modTemplateVar getRender input"
Aside from changing the wording and adding context to make it a bit clearer what's the problem, this also moves it into the conditional. Before, the message would get logged a lot for each first TV of a certain type due to the way the foreach handles registered render methods. Now it only gets logged when a deprecated flat file is used.

@Mark-H Mark-H added this to the v2.7.1 milestone Nov 28, 2018

@Mark-H Mark-H requested a review from opengeek as a code owner Nov 28, 2018

@@ -443,6 +442,7 @@ public function getRender($params,$value,array $paths,$method,$resourceId = 0,$t
/* 2.1< backwards compat */
$renderFile = $path.$type.'.php';
if (file_exists($renderFile)) {
$this->xpdo->deprecated('2.2.0', '', '<2.2 style template variable flat render file ' . $renderFile . ', for TV ' . $this->get('name'));

This comment has been minimized.

Copy link
@Jako

Jako Nov 29, 2018

Collaborator

Don't include the deprecated since version twice and add a message, who has to fix that issue:

$this->xpdo->deprecated('2.2.0', '', 'Using old style template variable flat render file ' . $renderFile . ', for TV ' . $this->get('name') . '. Please contact the author of the template variable type to change this code part. );

This comment has been minimized.

Copy link
@Mark-H

Mark-H Dec 3, 2018

Author Collaborator

Do we need to include "Please contact the author of blabla" in all the deprecation notices? Seems unnecessary to me.

This comment has been minimized.

Copy link
@Mark-H

Mark-H Dec 5, 2018

Author Collaborator

Are you happy with the updated message @Jako?

@Jako

Jako approved these changes Dec 6, 2018

@Alroniks

This comment has been minimized.

Copy link
Collaborator

Alroniks commented Dec 6, 2018

Need your help with approval @opengeek

@Alroniks Alroniks self-assigned this Jan 8, 2019

@Alroniks Alroniks merged commit 40a6806 into modxcms:2.x Jan 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

Alroniks added a commit that referenced this pull request Jan 8, 2019

Improve deprecated notice for "Old modTemplateVar getRender input" #1…
…4162

* upstream/pr/14162:
  Don't duplicate the version number in deprecated notice
  Improve deprecated notice for "Old modTemplateVar getRender input"

@Mark-H Mark-H deleted the Mark-H:fix-deprecated-tvrender branch Jan 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.