Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fixed ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO macro#2523

Closed
Fleshgrinder wants to merge 5 commits into
php:masterphp/php-src:masterfrom
Fleshgrinder:bugfix/arg-info-return-obj-macroCopy head branch name to clipboard
Closed

Fixed ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO macro#2523
Fleshgrinder wants to merge 5 commits into
php:masterphp/php-src:masterfrom
Fleshgrinder:bugfix/arg-info-return-obj-macroCopy head branch name to clipboard

Conversation

@Fleshgrinder
Copy link
Copy Markdown
Contributor

@Fleshgrinder Fleshgrinder commented May 14, 2017

The macro was expanding to a wrong macro and thus lead to compile errors upon usage. Changed it to expand to the correct macro, and added Doxygen documentation to help new users.

This is a pure internals fix, hence, I doubt that we want a real PHP bug report for this.

The macro was expanding to a wrong macro and thus leads to compile errors upon
usage. Changed it to expand to the correct macro, and added Doxygen
documentation to help new users.
Thanks Visual Studio ;(
@Fleshgrinder Fleshgrinder mentioned this pull request May 24, 2017
@nikic
Copy link
Copy Markdown
Member

nikic commented May 24, 2017

The technical change here can be easily merged, but the added documentation will require an internals discussion. We do not currently use Doxygen and as such we also do not use Doxygen-style comments anywhere in the codebase. If we want to start making use of Doxygen, this needs to be discussed first.

@Fleshgrinder
Copy link
Copy Markdown
Contributor Author

Fleshgrinder commented May 24, 2017

I can change the style to whatever is used, the thing is, there is no documentation. However, having it would greatly improve the accessibility of internals for others.

Update: Opened discussion on internals.

@krakjoe
Copy link
Copy Markdown
Member

krakjoe commented May 25, 2017

Please link to discussion for reference.

@Fleshgrinder
Copy link
Copy Markdown
Contributor Author

Internals Discussion: http://news.php.net/php.internals/99140

@remicollet
Copy link
Copy Markdown
Member

Applied in 66e5dc5
without the docblock stuff with is under discussion

@remicollet remicollet closed this Jun 30, 2017
@Fleshgrinder Fleshgrinder deleted the bugfix/arg-info-return-obj-macro branch July 1, 2017 17:50
basvanbeek added a commit to census-instrumentation/opencensus-php that referenced this pull request Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.