std.curl: document public methods #4314
Conversation
| } | ||
| } | ||
|
|
||
| // Workaround bug #2458 |
There was a problem hiding this comment.
Until this struct is moved into byLineAsync, this comment should stay
There was a problem hiding this comment.
the bug is fixed, but afaik having struct outsides is currently better because it avoids the long names. Ideally we would disallow access to it, but that would be yet another deprecation cycle?
There was a problem hiding this comment.
Ok, the comment can be removed and this can be moved inside when https://issues.dlang.org/show_bug.cgi?id=15831 is fixed.
Maybe add this note:
// @@@@BUG 15831@@@@
// this should be inside byLineAsync
There was a problem hiding this comment.
as far as i understood 15831 it just increases the binary size and people are already working against this problem, so we don't need to let us be limited by this. What do other people think?
btw as this symbol wasn't documented and had an explicit warning against calling it, i think it's okay to set it private or move in the function.
There was a problem hiding this comment.
as far as i understood 15831 it just increases the binary size
It's not just that. Your link times also increase exponentially with the symbol sizes. Also, debugging with symbols that are > 2000 characters sucks.
and people are already working against this problem
People are working on compression of symbol names, which is a bandaid and not solution. You still end up with slower compile times, exponential symbol size, and an extra step to debugging.
i think it's okay to set it private or move in the function.
Sure, set it to private, I'd argue that the above mentioned comment should be there as well.
wilzbach
force-pushed
the
public_document_curl
branch
2 times, most recently
from
148ddc5 to
63ffbe0
Compare
|
@JackStouffer actually all comments were addressed - Github doesn't show that because the comments are marked for removed lines. |
std/net/curl.d
Outdated
| version(StdDdoc) import std.stdio; | ||
|
|
||
| /// | ||
| extern (C) void exit(int); |
There was a problem hiding this comment.
Ehm? It just "imports" libc's exit function AFAICT, no need to make it public, rather it should be private
wilzbach
force-pushed
the
public_document_curl
branch
from
63ffbe0 to
b89adfc
Compare
|
LGTM |
|
Auto-merge toggled on |
3 participants
follow-up to #4312 -
stoppedwas exposed to the public - I wasn't sure whether this was on prupose, but to be sure I added a wrapper around it.