std.algorithms: document public methods #4312
Conversation
| // @@@BUG@@@ | ||
| //return haystack[$ .. $]; | ||
| return haystack[haystack.length .. haystack.length]; | ||
| return haystack[$ .. $]; |
There was a problem hiding this comment.
it seems to me that this bug is fixed?
wilzbach
force-pushed
the
public_document_algorithms
branch
from
3fca1c8 to
5689a48
Compare
| @@ -1938,7 +1938,7 @@ if (isRandomAccessRange!R1 && isForwardRange!R2 && !isBidirectionalRange!R2 && | |||
|
|
|||
| // Internally used by some find() overloads above. Can't make it | |||
| // private due to bugs in the compiler. | |||
| /*private*/ R1 simpleMindedFind(alias pred, R1, R2)(R1 haystack, R2 needle) | |||
| private R1 simpleMindedFind(alias pred, R1, R2)(R1 haystack, R2 needle) | |||
There was a problem hiding this comment.
it seems that this bug is fixed?
|
@burner IIRC you're the one who made all of the |
|
@JackStouffer I don't recall this. Anyway, if they are public functions and are supposed to be public then the should be documented. |
|
this PR is mixing documentation with general maintenance and visibility changes. Could you split that into multiple PRs. That would make reviewing easier and thereby make pulling or closing faster. |
| @@ -1319,7 +1319,7 @@ private struct FilterBidiResult(alias pred, Range) | |||
| } | |||
| } | |||
|
|
|||
| // group | |||
| /// group | |||
| struct Group(alias pred, R) if (isInputRange!R) | |||
There was a problem hiding this comment.
This should be under the same docs as its factory function. Same goes for all of the public range structs.
There was a problem hiding this comment.
Yeah, and you should move it to under the factory function. No one should be using the struct directly, but since it's public we can't make it a static struct without deprecation cycles that are of questional value anyway.
@burner so that I understand right - setting accidentally exposed symbols to |
|
@wilzbach yes, because it is a bugfix and not a documentation change. As said, this will speed the whole process. The little extra time you will spend branching you will make back double/tripple during review. |
| struct Permutations(Range) | ||
| if (isRandomAccessRange!Range && hasLength!Range) | ||
| { | ||
| size_t[] indices, state; | ||
| Range r; | ||
| private size_t[] indices, state; |
There was a problem hiding this comment.
You need to deprecate these, perferably in a different PR. See https://github.com/dlang/phobos/pull/3950/files
wilzbach
force-pushed
the
public_document_algorithms
branch
from
5689a48 to
6793405
Compare
wilzbach
force-pushed
the
public_document_algorithms
branch
from
6793405 to
b8f17e2
Compare
done - this PR now only contains documentation changes and as @JackStouffer suggested I moved undocumented |
|
LGTM |
ping |
|
Based on the recent discussions on Voldemort types, I wonder, shouldn't these actually be Voldemort? I wouldn't make them so, because this can bloat the binary incredibly fast. But I don't know if we should expose the types as public API. I'm working on a possible solution to the bloat that would allow us to have the types as Voldemort but not with the bloat. |
|
LGTM |
|
Auto-merge toggled on |
This just adds them to the documentation output - they are already publicly exposed and would need a deprecation cycle if we make them private/Voldemort again,. |
|
(as an aside, github is really borked for me today, it doesn't show all the discussion). It's a much easier deprecation to remove undocumented public symbols than it is to remove public API. |
|
In any case, we have until the next major release to decide. |
|
I did just turn auto merge on. but github does not display it. Lets see what explodes and then take action |
Here's a small project that I intend to do when I am on the road - document the public methods in Phobos or set them private etc ;-)
As you can imagine, many parts in Phobos aren't properly ddoced - so let's do this bit by bit.
If someone wants to help out, here's the current output of dscanner.
There are some fixes which might be controversial - I will mark them so.