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

Set the error reporting to include E_USER_WARNING #9201

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

tburry
Copy link
Contributor

@tburry tburry commented Aug 19, 2019

I want to be able to trigger warnings for some things that we plan to change in the future, but are uncertain of the production implications.

This change will send E_WARNING and E_USER_WARNING errors to the log rather than discarding them.

Note: I added E_WARNING to the list of warnings we report because it seems like the right thing to do, but that may end up being too noisy. I feel like I remember some really dumb things escalating to E_WARNING in PHP, but I just can't remember what.

I want to be able to trigger warnings for some things that we plan to change in the future, but are uncertain of the production implications.

This change will send E_USER_WARNING errors to the log rather than discarding them.
@tburry tburry requested a review from DaazKu August 19, 2019 20:18
@charrondev
Copy link
Contributor

charrondev commented Aug 19, 2019

If we're doing this we need to take an aggressive stance to cleaning up deprecated function calls in our code base. Currently we have enough that we added a separate \Vanilla\Utility\Deprecate::log that does does write the deprecations to the log (and the error log).

If we are (and by https://github.com/vanilla/dev-inter-ops/issues/16 it looks like it will be), I'm all for it.

@tburry
Copy link
Contributor Author

tburry commented Aug 20, 2019

Are the two necessarily related?

I want a warning so that we can model runtime behavior in production. This is something we can't easily do another way.

Deprecated function use should be able to be modeled with static analysis. Furthermore, the use of a deprecated function doesn't necessarily impact our product. It's usually just a function we no longer want to use. My guess the fact that we haven't made a huge effort to remove deprecated functions in our core repos means it's not actually as high a priority to the team.

@danni-stark danni-stark self-requested a review August 22, 2019 13:16
@charrondev charrondev merged commit de354bc into master Aug 22, 2019
@charrondev charrondev deleted the feature/log-warnings branch August 22, 2019 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants