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

check for PHP version before checking for (get_magic_quotes_gpc() #107

Conversation

mambax7
Copy link
Contributor

@mambax7 mambax7 commented Nov 22, 2023

No description provided.

Copy link
Collaborator

@geekwright geekwright left a comment

Choose a reason for hiding this comment

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

To my eyes, this is more regression than improvement. The function_exists() documents exactly what is being checked, unlike the PHP version which could be checking for any of thousands of things.

The existing test works as intended, and all of the magic quotes crap should be removed in the next xmf (and XOOPS) version.

Not worth a new xmf build version from my viewpoint.

@mambax7
Copy link
Contributor Author

mambax7 commented Dec 4, 2023

The problem is that I was still getting error notifications that the magic method doesn't exist, and that's what I wanted to eliminate with this PR. If you know a better way to avoid these annoying notifications, please share I'll rewrite it and re-submit it.

@geekwright
Copy link
Collaborator

geekwright commented Dec 4, 2023

The problem is that I was still getting error notifications that the magic method doesn't exist, and that's what I wanted to eliminate with this PR. If you know a better way to avoid these annoying notifications, please share I'll rewrite it and re-submit it.

Please clarify, is this a PHP runtime error or a code analysis generated error?

@mambax7
Copy link
Contributor Author

mambax7 commented Dec 4, 2023

PHP Runtime notification, I.e. the code was working, but I was getting these annoying notifications

@mambax7 mambax7 closed this Nov 27, 2024
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.

3 participants