-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Remove proto comments #5758
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
Remove proto comments #5758
Conversation
@kocsismate, better now? :) |
@MaxSem Really nice! :) I had a quick look, and the PR mostly looked good to me! The only very small gotcha I saw was that the description is repeated in case of functions that have multiple aliases (e.g. in intl). I don't think you have to address these now, so I'm perfectly ok with keeping them for now. In the meanwhile, I'll convert my PR to remove the protos from tests as well. |
ext/intl/collator/collator_attr.c
Outdated
/* {{{ proto int collator_get_attribute( Collator $coll, int $attr ) | ||
* Get collation attribute value. | ||
/* {{{ * Get collation attribute value. }}} */ | ||
/* {{{ * Get collation attribute value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try to avoid these odd cases at least.
649c2ed
to
960589c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on a quick scroll through this looks mostly fine. One recurring issue that should be fixed before merging is that /* {{{ *
should be just /* {{{
. Other minor issues can be fixed up after the fact, to avoid bit rot.
ext/intl/converter/converter.c
Outdated
@@ -103,8 +103,7 @@ static void php_converter_default_callback(zval *return_value, zval *zobj, zend_ | |||
} | |||
/* }}} */ | |||
|
|||
/* {{{ proto void UConverter::toUCallback(int $reason, | |||
string $source, string $codeUnits, | |||
/* {{{ string $source, string $codeUnits, | |||
int &$error) */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't right.
They've been made obsolete by .stub.php files and are often outdated as they're not the source of truth.
I had a look at the new commit, and it looked good. So I took the liberty and merged this in order not to face any conflicts. @nikic If you find any formatting issues, I can also help sort them out. |
They've been made obsolete by .stub.php files and are often outdated as they're not the source of truth.