Improve parameter names in ext/curl #6155
Conversation
|
|
||
| function curl_multi_init(): CurlMultiHandle {} | ||
|
|
||
| function curl_multi_remove_handle(CurlMultiHandle $multi_handle, CurlHandle $handle): int {} | ||
|
|
||
| function curl_multi_select(CurlMultiHandle $multi_handle, float $timeout = 1.0): int {} | ||
|
|
||
| function curl_multi_strerror(int $error_number): ?string {} | ||
| function curl_multi_strerror(int $error_code): ?string {} |
There was a problem hiding this comment.
| function curl_multi_strerror(int $error_code): ?string {} | |
| function curl_multi_strerror(int $errno): ?string {} |
to use the same terminology as all the curl_*_errno APIs?
There was a problem hiding this comment.
I'm not sure I like this idea more than having a nice $error_code param name ^^ + the manual mentions error code in quite a few places. Nevertheless, I'm open for asking around for other opinions :)
There was a problem hiding this comment.
"errno" is a common enough abbreviation, but honestly I'd prefer the full "error_code". Especially if that's what's in the manual and what we use elsewhere.
There was a problem hiding this comment.
Especially if that's what's in the manual and what we use elsewhere.
For this particular function, the manual uses $errornum. Checking statistics for current stubs parameter names, we have the following names in use:
"errno": 8,
"error": 6,
"error_number": 3,
"error_code": 2,
"errorcode": 2,
Some of them might have different meaning though.
From this I conclude ... I have no idea.
There was a problem hiding this comment.
Anyway, I'm fine with $error_code here. The one place we really need to use $errno is APIs like pcntl_strerror/posix_strerror/socket_strerror that deal with the C errno concept. In other places I don't see a big problem with $error_code.
There was a problem hiding this comment.
For this particular function, the manual uses $errornum.
Ah sorry, I wasn't very clear. So the manual uses "error code" in the descriptions. What I tried to say with this is that I don't see any reason to make the param name any different from what we normally use in other places, since it's a clear and short term.
ext/curl/curl.stub.php
Outdated
| /** @param int|null $msgs_in_queue */ | ||
| function curl_multi_info_read(CurlMultiHandle $multi_handle, &$msgs_in_queue = null): array|false {} | ||
| /** @param int $queued_message_count */ | ||
| function curl_multi_info_read(CurlMultiHandle $multi_handle, &$queued_message_count = null): array|false {} |
There was a problem hiding this comment.
Descriptive, but a bit long. Would simply $message_count be sufficient? Or int $queued_messages
kocsismate
force-pushed
the
curl-names
branch
from
5103008 to
35cc06b
Compare
No description provided.