CI https://ci.nodejs.org/job/node-test-commit-light/251/

assert.doesNotThrow looks widely used, btw. Not entirely sure how many of those are using the built-in assert lib, though.

@ChALkeR I am also pretty sure people use it. The question is though if we want to encourage new people to use a pretty useless API that should be replaced with comments.

And because it is used right now, I think a doc only deprecation is best in this case.

@BridgeAR Note that chai implements this differently: http://chaijs.com/api/assert/#method_doesnotthrow

tape seems to roughly match Node.js impl: https://github.com/substack/tape/blob/master/lib/test.js#L515

@ChALkeR yes, the chai implementation has a use case. Even though I am not not sold on testing explicitly against a type of error / message and accept all others.

Even without deprecating, we could add a note to assert.doesNotThrow() documentation that it's not very useful and is there for consistency.

@ChALkeR I like that idea.

@cjihrig idea is also ok. Can you send an alternate PR @BridgeAR?

@cjihrig

If people want to use this API for whatever reason, that's their choice.

That holds true even after this documentation only deprecation. There will not be any warning what so ever. But people who try to find out how the assert module works will clearly see that this is actually not that useful. Only adding a regular comment does not solve the same as well because it is not that obvious.

I tried to find a single use case but I could not find any. Not with any argument combination. That is why I suggest to doc-only deprecate the API.

The chai implementation works differently and even though I do not know a good reason for that API either, I think that it works the way I would have expected it to work here. But changing it to that implementation is not possible as that would be a hard breaking change.

That holds true even after this documentation only deprecation. There will not be any warning what so ever.

Right, but a docs deprecation sort of means we're planning to runtime deprecate / remove it one day.

Right, but a docs deprecation sort of means we're planning to runtime deprecate / remove it one day.

I am not aware that this is written down anywhere. I do not see that as a necessary subsequent step. I personally would not want to remove this API (probably ever) because it is used too often. A runtime deprecation is probably also not in place for multiple more years out of my perspective. That might be evaluated again at some point of time but even then: to change the deprecation from documentation-only to something else someone has to open a PR for it and it would probably be denied.

Therefore I would still like to land this as is.

Ping @cjihrig

I'm still -1 on an official docs deprecation, but am fine with something like @ChALkeR suggested.

@cjihrig as far as I understand your reasons you were mainly against this ever becoming a Runtime deprecation, right? Because I believe there should be other ways to guarantee that than not deprecating this at all.

LGTM with @ChALkeR’s comment

Edit, to maybe clarify a bit: API reference documentation is supposed to be more informative than instructive. It’s the same reason we avoid “you” in our documentation.

Edit, to maybe clarify a bit: API reference documentation is supposed to be more informative than instructive. It’s the same reason we avoid “you” in our documentation.

Why? I've never really understood this.

If you pick a random page from the std library of another language, they all seem to use "you" indiscriminately:

• Rust
• Python
• Ruby
• Go
• JS (MDN)

Fundamentally we're instructing users of Node in how we think they should use the APIs we provide. Making the tone more formal (e.g. making everything passive) doesn't really change that.

Comments addressed.

Lite-CI https://ci.nodejs.org/job/node-test-pull-request-lite/232/

Ping @jasnell @addaleax

Landed in 8b69d4a

I removed the semver-minor label as that does not apply anymore since it is only a comment.