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

Update config.js.php processor to be class based #14161

Merged
merged 2 commits into from
Dec 5, 2018

Conversation

Mark-H
Copy link
Collaborator

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

What does it do?

In #14160 I made the deprecated message a bit more useful for identifying offending processors, which immediately pointed to the config.js.php which is called on each manager request as loading the config into a JS object. So, I wrapped it into a modProcessor class to resolve that, and added a migration to remove the old processor file when running the 2.7.1 setup.

Why is it needed?

The deprecated log message on each manager request will not only get annoying fast, but as it's not very useful until #14160, this particular instance may encourage people to disable the deprecated logging preventing them from seeing other deprecated usages that need to be fixed.

Related issue(s)/PR(s)

#14160

@Mark-H Mark-H added this to the v2.7.0 milestone Nov 28, 2018
@Mark-H Mark-H requested a review from opengeek as a code owner November 28, 2018 21:23
@Mark-H Mark-H modified the milestones: v2.7.0, v2.7.1 Nov 28, 2018
@alroniks alroniks self-assigned this Dec 3, 2018
@alroniks alroniks added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Dec 3, 2018
@alroniks alroniks merged commit d6861fa into modxcms:2.x Dec 5, 2018
alroniks pushed a commit that referenced this pull request Dec 5, 2018
* upstream/pr/14161:
  Back to the future! (Fix referred version)
  Update config.js processor to be class based, and remove old config.js.php in a setup migration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants