Fix Issue 8472 - Replace walkLength() with an improved count()

[RazvanN7]

I am not sure how the deprecation cycle works, but I deprecated the first walkLength overload which is obsolete. I don't know if the walkLength overload which walks up to a given number of elements should be deprecated too. I think that the best way is to deprecated one overload of walkLength and keep the other.

[dlang-bot]

Thanks for your pull request, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Fix	Bugzilla	Description
✓	8472	Replace walkLength() with an improved count()

[wilzbach]

Your is swallows all errors (even your typos), but comparing functions doesn't work in D, e.g.:

void main(string[] args)
{
    import std.functional;
    static assert(is(typeof(unaryFun!("true") == unaryFun!("true"))));
}

Regarding the specialization, you should see #4265 (comment) and its final version (#5001).
tl;dr: use overloads, not strings

[RazvanN7]

I guess in this case I should create an overload which doesn't receive a string function, but calls the one I just modified

[wilzbach]

You should use the deprecated string, so it's easy to grep for:

// Explicitly undocumented. It will be removed in July 2018. @@@DEPRECATED_2018-07@@@

And of course, undocument or state in the document with $(RED) that it's deprecated.

[jmdavis]

Also, that deprecated string is for symbols that have already been through a year of deprecation and are now undocumented. So, even if we deprecated walkLength (which IMHO, we shouldn't), this would not be the message to use. It would have to be one from a symbol that's still documented with a ddoc comment.

[JackStouffer]

@RazvanN7 See https://issues.dlang.org/show_bug.cgi?id=11429

[RazvanN7]

I suggest keeping the remaining walkLength and deprecating the one that count can take care off

[RazvanN7]

I'm guessing the jenkins failure is due to the fact that the use of deprecated functions is forbidden in phobos?

[wilzbach]

I'm guessing the jenkins failure is due to the fact that the use of deprecated functions is forbidden in phobos?

No it's simply broken atm (or has a lot of transient failures), however CircleCi isn't:

std/range/primitives.d(1656:6)[warn]: Public declaration 'walkLength' is undocumented.

due to the fact that the use of deprecated functions is forbidden in phobos?

Wishful thinking -> #5546

[RazvanN7]

@wilzbach seems kind of awkward to document deprecated functions.

[jmdavis]

Please do not deprecate walkLength. If count can do what what walkLength does, then fine. But walkLength has been around since the beginning of ranges in Phobos, and it will break a lot of code if folks are forced to change their code, because walkLength has been deprecated. This seems like a reasonable optimization to count, but I don't think that anything should be done to walkLength because of it.

[wilzbach]

@wilzbach seems kind of awkward to document deprecated functions.

-> dlang-community/D-Scanner#492

[wilzbach]

@RazvanN7 I suggest to split this PR and submit the optimization for count in another as that one isn't controversial and can be merged quickly.

[jmdavis]

Also, if walkLength isn't being deprecated, it would be better for count to simply call walkLength in the new overload rather than reimplementing it, because there's less code duplication that way.

[andralex]

@RazvanN7 deprecation of anything is a tall order and should be confined to its own pull request (as opposed to mixed with other work). I'll close this and comment in the other.