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 consistency in PHP version ifdef usage #56

Open
joshuabenuck opened this issue Dec 17, 2020 · 4 comments
Open

Improve consistency in PHP version ifdef usage #56

joshuabenuck opened this issue Dec 17, 2020 · 4 comments
Labels
enhancement New feature or request internal Moved to NR Tracking System

Comments

@joshuabenuck
Copy link
Contributor

Summary

The version specific PHP ifdefs should refer to the named version only and not the named version or higher.

Desired Behavior

PHP7 should only refer to PHP version 7, not version 8 in order to be consistent with the PHP5 and PHP8 ifdefs.

Possible Solution

We may also want to consider adding something like a PHP7_PLUS ifdef in order to avoid the more complex zend module version comparison ifdef used now.

Additional context

PR #49 is where the PHP8 ifdef was first added and when the inconsistency was introduced (since there is now a version greater than PHP 7.x).

@joshuabenuck joshuabenuck added the enhancement New feature or request label Dec 17, 2020
@stale stale bot added the wontfix This will not be worked on label Jan 16, 2021
@newrelic newrelic deleted a comment from stale bot Jan 19, 2021
@stale stale bot removed the wontfix This will not be worked on label Jan 19, 2021
@Miriam-R Miriam-R added this to the PHP 8 Support milestone Feb 10, 2021
@Miriam-R
Copy link

Part of #34 Agent Tests

@zsistla
Copy link
Contributor

zsistla commented Mar 3, 2021

Unless there is a specific ZEND_8_0_X_API_NO or another ZEND_7_x_API_NO block,

Change #ifdef PHP7

to be:
#if ZEND_MODULE_API_NO >= ZEND_7_0_X_API_NO /* PHP 7.0+ */

This will avoid confusion and increase clarity because the majority of those blocks also apply to PHP8.

@insanebits
Copy link

Is there any progress on this?

@zsistla zsistla removed this from the PHP 8 Support milestone Jun 3, 2021
@zsistla
Copy link
Contributor

zsistla commented Jun 3, 2021

Hello @insanebits, and thank you for your interest! This issue is in our backlog to be worked on, but currently has no scheduled date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal Moved to NR Tracking System
Projects
None yet
Development

No branches or pull requests

5 participants