Jonas Drewsen wrote:
>Hi,
>
> After all the comments from last review I've refactored the curl
>wrapper and it is ready for a new review.
>
>David Nadlinger was handling the last review so I guess it would make
>sense if he run this one as well if he wants to.
>
>Code:
>https://github.com/jcd/phobos/blob/curl-wrapper/etc/curl.d
>
>Docs:
>http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
>
>Regards,
>Jonas
Looks great so far!
Some minor nitpicks:
* Examples: "new Http()" is used 4 times in the examples, but as Http is
a struct, I see no reason to create it using new?
* Move
----------------------------------------------
// Get using an line range and proxy settings
auto client = new Http();
client.proxy = "1.2.3.4";
foreach (line; byLine("d-p-l.org", client))
writeln(line);
----------------------------------------------
from the first to the second example. (The sentence "For more control
than the high level functions provide, use the low level API:" sounds
like everything shown up to that point was the high level api, so that
one example should be moved below that sentence)
* smtp.perform; --> smtp.perform();
* Please set default timeouts for the simple API. Nothing is more
annoying than a application locking up because of internet loss..
* Speaking of timeouts, I'd like a special TimeoutException, as it's
easy to recover from these errors.
* At least the high-level API should really be @safe (or @trusted).
Adding those qualifiers to the low level API would also be good, but
that can always be done after merging into phobos.
* Some functions could be nothrow? @property StatusLine statusLine()
for example, maybe some more.
--
Johannes Pfau
