New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: remove assertions about assert #11113

Closed
wants to merge 1 commit into
from

Conversation

@Trott
Member

Trott commented Feb 2, 2017

The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc assert

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.
@TimothyGu

LGTM. I, for one, never knew assert was meant to be internal until this PR.

@danbev

danbev approved these changes Feb 2, 2017

@joyeecheung

This comment has been minimized.

Show comment
Hide comment
@joyeecheung

joyeecheung Feb 2, 2017

Member

@TimothyGu Me neither. In fact I know quite a few folks who really love power-assert, which depends on assert and the basic idea is like, "why do we need other modules if assert in Node.js core is pretty much all we need"?

Member

joyeecheung commented Feb 2, 2017

@TimothyGu Me neither. In fact I know quite a few folks who really love power-assert, which depends on assert and the basic idea is like, "why do we need other modules if assert in Node.js core is pretty much all we need"?

@mscdex mscdex referenced this pull request Feb 2, 2017

Closed

Bot failed to label PR #88

@gibfahn

gibfahn approved these changes Feb 2, 2017

@jasnell

jasnell approved these changes Feb 2, 2017

The API for the `assert` module is [Locked][]. This means that there will be no
additions or changes to any of the methods implemented and exposed by
the module.
additions or changes to any of the methods implemented and exposed by the

This comment has been minimized.

@thefourtheye

thefourtheye Feb 2, 2017

Contributor

We can simply remove this line, as the linked page covers this.

@thefourtheye

thefourtheye Feb 2, 2017

Contributor

We can simply remove this line, as the linked page covers this.

@ChALkeR

ChALkeR approved these changes Feb 2, 2017

@italoacasas

This comment has been minimized.

Show comment
Hide comment
Member

italoacasas commented Feb 3, 2017

Landed 42e73ae

@italoacasas italoacasas closed this Feb 3, 2017

italoacasas added a commit that referenced this pull request Feb 3, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: #11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

italoacasas added a commit that referenced this pull request Feb 4, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: #11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@thefourtheye

This comment has been minimized.

Show comment
Hide comment
@thefourtheye

thefourtheye Feb 4, 2017

Contributor

@italoacasas I think my comment may have been overlooked...

Contributor

thefourtheye commented Feb 4, 2017

@italoacasas I think my comment may have been overlooked...

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Feb 4, 2017

Member

@thefourtheye I think it should be fine do to address that in another PR, it wasn’t really part of the changes here anyways

Member

addaleax commented Feb 4, 2017

@thefourtheye I think it should be fine do to address that in another PR, it wasn’t really part of the changes here anyways

@thefourtheye

This comment has been minimized.

Show comment
Hide comment
@thefourtheye

thefourtheye Feb 4, 2017

Contributor

@addaleax Fair enough...

Contributor

thefourtheye commented Feb 4, 2017

@addaleax Fair enough...

italoacasas added a commit to italoacasas/node that referenced this pull request Feb 14, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: nodejs#11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

KryDos added a commit to KryDos/node that referenced this pull request Feb 25, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: nodejs#11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

jasnell added a commit that referenced this pull request Mar 7, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: #11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

jasnell added a commit that referenced this pull request Mar 7, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: #11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

MylesBorins added a commit that referenced this pull request Mar 9, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: #11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

@MylesBorins MylesBorins referenced this pull request Mar 9, 2017

Merged

v6.10.1 proposal #11759

MylesBorins added a commit that referenced this pull request Mar 9, 2017

doc: remove assertions about assert
The assert docs have some language that suggests that we don't want bug
fixes. We do. Send in bug fixes, please. (Just no new API features.)
We'd love to not have assert in core at all, but that ship has sailed.
It's here to stay. Let's at least make it not have surprising behaviors.
Because we want good things for our users.

PR-URL: #11113
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>

@MylesBorins MylesBorins referenced this pull request Mar 9, 2017

Merged

v4.8.1 proposal #11760

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment