Added Laminas 3.x support. #36
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ok jenkins |
agent/fw_laminas3.c
Outdated
| #include "util_memory.h" | ||
|
|
||
| /* | ||
| * Laminas is a rebranding of Zend, but the logic remains the same, |
There was a problem hiding this comment.
Looks like there's an extra space here before the Laminas comment
agent/lib_zend_http.c
Outdated
| nr_php_wrap_user_function(NR_PSTR(http_client_request), | ||
| nr_zend_http_client_request TSRMLS_CC); | ||
| } | ||
| } |
agent/lib_zend_http.c
Outdated
Comment on lines
33
to
37
| static char library_name[50] = "Zend"; | ||
| static char curl_adapter_typename[50] = "Zend_Http_Client_Adapter_Curl"; | ||
| static char uri_http_typename[50] = "Zend_Uri_Http"; | ||
| static char http_client[50] = "Zend_Http_Client"; | ||
| static char http_client_request[50] = "Zend_Http_Client::request"; |
There was a problem hiding this comment.
I wonder if we can do this by freeing and allocating new memory for the laminas naming scheme in nr_laminas_http_enable(), instead of copying the strings over?
|
Multiverse tests pass quite nicely. |
kneitinger
previously approved these changes
Nov 24, 2020
There was a problem hiding this comment.
This looks and functions nicely!
Transparency of approval: I committed to this PR, but only cosmetic ordering of values, and the feedback that @Fahmy-Mohammed left.
Fahmy-Mohammed
previously approved these changes
Nov 24, 2020
agent/lib_zend_http.c
Outdated
Comment on lines
46
to
50
| char * library_name = LIB_NAME_Z; | ||
| char * curl_adapter_typename = CURL_ADAPTER_Z; | ||
| char * uri_http_typename = URI_HTTP_Z; | ||
| char * http_client = HTTP_CLIENT_Z; | ||
| char * http_client_request = HTTP_CLIENT_REQUEST_Z; |
agent/fw_hooks.h
Outdated
| @@ -27,6 +27,7 @@ extern void nr_drupal_enable(TSRMLS_D); | |||
| extern void nr_drupal8_enable(TSRMLS_D); | |||
| extern void nr_joomla_enable(TSRMLS_D); | |||
| extern void nr_kohana_enable(TSRMLS_D); | |||
| extern void nr_fw_laminas3_enable(TSRMLS_D); | |||
There was a problem hiding this comment.
Could we change the name here for consistency. nr_fw_laminas3_enable -> nr_laminas3_enable
kneitinger
dismissed stale reviews from Fahmy-Mohammed and themself
via
c5b17d3
Fahmy-Mohammed
approved these changes
Nov 24, 2020
kneitinger
approved these changes
Nov 24, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Duplicated the current
Zendsupport withLaminasnaming.See https://source.datanerd.us/php-agent/multiverse/pull/56 for multiverse testing.