Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.curlConnect was renamed to curlInitialize in #336043: Simpletest speedup: skip call to curlExec() in curlConnect().
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | curl-exec_1.patch | 1.31 KB | mr.baileys |
| #6 | curl-exec.patch | 1.22 KB | mr.baileys |
| #4 | curl-exec.patch | 1.22 KB | mr.baileys |
| #1 | curl-initialize.patch | 683 bytes | mr.baileys |
Comments
Comment #1
mr.baileysComment #2
mr.baileysMoving to the documentation queue.
Comment #3
jhodgdonGood catch!
The patch needs a bit of work...
The first line of function doc needs to be less than 80 characters. This one is too long.
Also, if you're going to fix up this function doc, there is another line in there that needs 80-character wrapping, and there should be a blank line between the @param and @return sections in the function doc header.
Also, "Performs a cURL exec" is not really up to doc standards -- I can see that from the function name, but what does it mean?
How about (following how curl_exec() is documented on php.net):
Initializes and executes a cURL session.
(I don't think there's much value in the "with the specified options" stuff being in the one-line description -- of course it's going to use its parameters?)
Comment #4
mr.baileysActually, the Doxygen formatting conventions say: one sentence on one line (should not, but can exceed 80 chars). I've seen some other summary lines in core exceed the 80 character limit. Not sure if this is still valid though, and since I agree with you that the "with specified options" part can go, this is not an issue in this case.
Better, but "executing a session" sounds weird to me. What about "Initializes and executes a cURL request"?
Also added in a @see reference to the list of valid options for $curl_options, a @see reference to the curlInitialize function and slightly reworded the return info.
Comment #5
jhodgdonMy suggestion of "execute a session" came from the PHP.net documentation of cURL, but I agree that "request" makes more sense to me as well.
So, this patch is almost OK. But you cannot/should not put a @see in the middle of a @param doc. @see creates a See Also section in the doc. Just use the word "See" as you would in normal English -- all function names with () after them automatically turn into links anyway, and I believe URLs do too.
Also, if possible, we're trying to remove English-centric php.net links (there's a separate issue on that). So can you remove the "en" from the link, and verify that the link still takes you to the right page via php.net redirection?
"call to curl_exec." -- curl_exec() should have () after it -- it's a function. On some API sites, this will automatically generate a link to php.net (although not on api.drupal.org).
"@see curlInitialize" -- isn't this a method? If so, needs () after it to generate the link.
Comment #6
mr.baileysThanks, new patch that fixes the remaining issues.
Comment #7
jhodgdonSo I tried the URL referenced in $curl_options. And now I'm confused about $curl_options.... How is it being used and passed into the PHP function? Is it an associative array? I think it needs some additional documentation.
Comment #8
mr.baileysFair enough. And to think this started out as a simple one-word fix ;)
New patch attached, see if it makes more sense. We could also include the fact that the options array is passed to curl_setopt_array(), but maybe that's going too much in detail.
Comment #9
jhodgdonLooks good to me! Thanks for your willingness to make the documentation better. :)
Comment #10
realityloop#8: curl-exec_1.patch queued for re-testing.
Comment #11
webchickWell that's never good. :)
Committed to HEAD! Thanks!