Hi,

   So after the last review of the etc.curl there were some requests for making it simpler.

After some thinking, refactoring and documentation I've come up with a somewhat changed API.

Before sending it for official review again I would really like some comments on the new API and if you think it is better or worse etc.

http://freeze.steamwinter.com/D/web/phobos/etc_curl.html

Thanks,
Jonas

Am 09.11.2011, 21:53 Uhr, schrieb Jonas Drewsen <jdrewsen@nospam.com>:

> Hi,
>
>     So after the last review of the etc.curl there were some requests for making it simpler.
>
> After some thinking, refactoring and documentation I've come up with a somewhat changed API.
>
> Before sending it for official review again I would really like some comments on the new API and if you think it is better or worse etc.
>
> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
>
> Thanks,
> Jonas

Amazing! D docs that look like they were made to be read by a human being! ;)
I like the accessibility, that gets you right to the point for any use case. I did not compare it to your previous version though.
The external links sometimes have four slashes http://// and the text "Authentication method equal to" looks incomplete and you wrote valie instead of value. Good luck with the next review!

On 11/9/2011 12:53 PM, Jonas Drewsen wrote:
> Before sending it for official review again I would really like some comments on
> the new API and if you think it is better or worse etc.
>
> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html

Thank you so much for doing this. I think it's a nice piece of work.

Some nitpicky comments:

* libcurl should be a link to where people can read up on what it is.
I suggest http://curl.haxx.se/libcurl
Point out that the user will need libcurl installed on their system in order to use etc.curl.

* Need a brief statement on why a D programmer should prefer using etc.curl rather than directly use libcurl.

* "ranges" should be a link to what a range is

* examples should include

    import etc.curl;

and any other needed, like std.stdio. In fact, they should be cut & paste complete examples that are compilable and runnable.

* Keep comment lines under 40 characters; use multiple lines for longer comments. (This is so things format better for small screens.) Try to format code so it doesn't need more than 40 lines. I tend to indent by only two spaces in documentation which helps.

* Remove all instances of the word "you". For example,

"If you need more control than the low high level functions provide, you'll have to use the low level API:"

rewrite as:

"For more control than the low high level functions provide, use the low level API:"

Note how much simpler and direct it is. "You" is almost always an unnecessary filler word, and frankly has no place in a reference work.

* For the same reason, get rid of "we":

"First, we create an instance" => "First, create an instance"
"We then set" => "Then set"

* derivate => derived

* "Connection type used when the url should be used to auto detect protocol"
 .. I have no idea what this sentence means.

* "HTTP/FTP upload from local file system." => Upload file from local
 file system using the HTTP or FTP protocol."
And which protocol would be used?

* "A string with the content of the resource pointer to by the url"
Missing period. Grammar problems, how about:
"A string containing the content of the resource pointed to by the url."

Most of these apply to the rest of the documentation.

Also, need more examples.

I tend to like references to things in other modules to be hyperlinks. For example, writeln should be $(LINK2 http://www.d-programming-language.org/phobos/std_stdio.html#writeln, writeln)

On 11/9/2011 4:24 PM, Walter Bright wrote:
> On 11/9/2011 12:53 PM, Jonas Drewsen wrote:
>> Before sending it for official review again I would really like some comments on
>> the new API and if you think it is better or worse etc.
>>
>> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
>
> Thank you so much for doing this. I think it's a nice piece of work.

I forgot to mention: I like the cheat sheet a lot.

Walter:

> Thank you so much for doing this. I think it's a nice piece of work.

I agree.
As they say, easy things should be easy and hard things should be possible.


> * libcurl should be a link to where people can read up on what it is.
> I suggest http://curl.haxx.se/libcurl
> Point out that the user will need libcurl installed on their system in order to
> use etc.curl.

Is the (just) Windows compiled version present in the DMD zip? I don't remember what was the decision on this (I'd like it to be present).

Bye,
bearophile

On Thu, 10 Nov 2011 01:25:29 +0100, Walter Bright <newshound2@digitalmars.com> wrote:

> On 11/9/2011 4:24 PM, Walter Bright wrote:
>> On 11/9/2011 12:53 PM, Jonas Drewsen wrote:
>>> Before sending it for official review again I would really like some comments on
>>> the new API and if you think it is better or worse etc.
>>>
>>> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
>>
>> Thank you so much for doing this. I think it's a nice piece of work.
>
> I forgot to mention: I like the cheat sheet a lot.

Indeed. I am tempted to argue all of our docs should have that.

On Wednesday, November 09, 2011 16:58 bearophile wrote:
> Is the (just) Windows compiled version present in the DMD zip? I don't
> remember what was the decision on this (I'd like it to be present).

Currently, no version of libcurl is included in the zip file. I don't believe that there has been any decision on what to do about it. But if we do end up including it, then that's one more reason to consider splitting up the zip file by OS.

- Jonathan M Davis

On Wednesday, November 09, 2011 17:05 Simen Kjærås wrote:
> On Thu, 10 Nov 2011 01:25:29 +0100, Walter Bright
> 
> <newshound2@digitalmars.com> wrote:
> > On 11/9/2011 4:24 PM, Walter Bright wrote:
> >> On 11/9/2011 12:53 PM, Jonas Drewsen wrote:
> >>> Before sending it for official review again I would really like some
> >>> comments on
> >>> the new API and if you think it is better or worse etc.
> >>> 
> >>> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
> >> 
> >> Thank you so much for doing this. I think it's a nice piece of work.
> > 
> > I forgot to mention: I like the cheat sheet a lot.
> 
> Indeed. I am tempted to argue all of our docs should have that.

It's essentially what std.algorithm does now, and std.container does something similar. I'd love to do something like that with std.datetime, but the fact that the anchors that ddoc generates have no concept of hiearchy (e.g. every year function in the file gets #year for its anchor rather than something like #Date.year, #DateTime.year, etc.). So, I couldn't provide any links in such a cheat sheat. I think that one or two people have said that they'd look into fixing it, but no one has actually fixed it yet. If no one else fixes it any time soon, I may be forced to go dig into ddoc to figure it out myself, which is definitely not where I want to be spending my time if I can avoid it.

- Jonathan M Davis

On 11/9/2011 5:13 PM, Jonathan M Davis wrote:
> On Wednesday, November 09, 2011 16:58 bearophile wrote:
>> Is the (just) Windows compiled version present in the DMD zip? I don't
>> remember what was the decision on this (I'd like it to be present).
>
> Currently, no version of libcurl is included in the zip file. I don't believe
> that there has been any decision on what to do about it. But if we do end up
> including it, then that's one more reason to consider splitting up the zip
> file by OS.

I don't want us to get caught in the rat race of "the version that comes with the compiler is out of date". libcurl has its own development cycle, and dmd should not be slaved to that.

On 11/9/11 6:24 PM, Walter Bright wrote:
> I tend to like references to things in other modules to be hyperlinks.
> For example, writeln should be $(LINK2
> http://www.d-programming-language.org/phobos/std_stdio.html#writeln,
> writeln)

That should be XREF actually.

Andrei