Curl wrapper review

[Jonas Drewsen]

Den 16-11-2011 00:08, Bernard Helyer skrev:
> I'll just post my thoughts here while they're fresh. It looks good. The
> documentation is what I'd expect from a Phobos module, as is the naming
> convention.
>
> auto _basicFtp(T)(const(char)[] url, const(void)[] sendData, Ftp client)

You're right. Should be private

> If you don't want people using it, shouldn't it be marked private instead
> of using the underscore for obscurity?
>
> private struct Pool(DATA)
> {
> private:
>
> You've marked private things as 'private foo;' everywhere else in the
> module, what's with the switch in styles for this struct? Also, as the
> whole struct is module private I'm not sure of the utility of marking
> members private. I guess it's a form of documentation.

Regarding the switch in I just think that I'd been doing lots of c++ 
coding that day. Anyway I think it is ok to use that style for small 
classes/structs that can fit on a single screen even though I haven't 
done that consistently either in this module. I'll change it to match 
the rest of the module.

Regarding the second question: I just think it is good style to mark up 
the private parts. And if someone copy/pastes it as a public struct 
it'll work as intended.

> But really, I'm grasping at straws. Even if the above were to remain, I
> would love to see this in Phobos yesterday. :)
>
> -Bernard.

Thanks for the comments.

/Jonas