doc: update the notable changes in 10.x #20316

BridgeAR · 2018-04-26T02:31:01Z

A couple entries were missing and one entry was not really relevant.

The assert.throws change is actually semver-minor but it did not land on 9.x yet, so 10.0.0 is the first release for it. So I guess it is also the place to note that change?

I am not sure if it makes a lot of sense to change these entries now but I thought I can just open the PR and see what others think.

If it should be updated: I guess the wording for these could be further improved.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

A couple entries were missing and one entry was not really relevant.

BridgeAR · 2018-04-26T02:31:45Z

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

Trott

Actualy changes LGTM. No opinion on whether it's a good idea to modify the notable changes list after-the-fact like this.

Trott · 2018-04-26T03:20:15Z

No opinion on whether it's a good idea to modify the notable changes list after-the-fact like this.

I guess @nodejs/release might have opinions?

vsemozhetbyt

With some possible nits)

vsemozhetbyt · 2018-04-26T03:32:01Z

doc/changelogs/CHANGELOG_V10.md

  * Calling `assert.ifError()` will now throw with any argument other than `undefined` or `null`. Previously the method would throw with any truthy value. [[`e65a6e81ef`](https://github.com/nodejs/node/commit/e65a6e81ef)]
  * The `assert.rejects()` and `assert.doesNotReject()` methods have been added for working with async functions. [[`599337f43e`](https://github.com/nodejs/node/commit/599337f43e)]
+  * Assertion errors will show a diff in case objects are used. [[`2d9e87695e`](https://github.com/nodejs/node/commit/2d9e87695e)]
+  * `assert.throws` accepts an object for comparison to the error. [[`2d374916eb`](https://github.com/nodejs/node/commit/2d374916eb)]


`assert.throws` -> `assert.throws()`?

vsemozhetbyt · 2018-04-26T03:34:52Z

doc/changelogs/CHANGELOG_V10.md

 * Async_hooks
  * Older experimental async_hooks APIs have been removed. [[`1cc6b993b9`](https://github.com/nodejs/node/commit/1cc6b993b9)]
 * Buffer
  * Uses of `new Buffer()` and `Buffer()` outside of the `node_modules` directory will now emit a runtime deprecation warning. [[`9d4ab90117`](https://github.com/nodejs/node/commit/9d4ab90117)]
  * `Buffer.isEncoding()` now returns `undefined` for falsy values, including an empty string. [[`452eed956e`](https://github.com/nodejs/node/commit/452eed956e)]
  * `Buffer.fill()` will throw if an attempt is made to fill with an empty `Buffer`. [[`1e802539b2`](https://github.com/nodejs/node/commit/1e802539b2)]
+  * Removed the `noAssert` argument from all `Buffer` read and write functions. [[`e8bb1f35df`](https://github.com/nodejs/node/commit/e8bb1f35df)]


`noAssert` argument was removed...?

vsemozhetbyt · 2018-04-26T03:36:18Z

doc/changelogs/CHANGELOG_V10.md

  * Calling `assert.ifError()` will now throw with any argument other than `undefined` or `null`. Previously the method would throw with any truthy value. [[`e65a6e81ef`](https://github.com/nodejs/node/commit/e65a6e81ef)]
  * The `assert.rejects()` and `assert.doesNotReject()` methods have been added for working with async functions. [[`599337f43e`](https://github.com/nodejs/node/commit/599337f43e)]
+  * Assertion errors will show a diff in case objects are used. [[`2d9e87695e`](https://github.com/nodejs/node/commit/2d9e87695e)]
+  * `assert.throws` accepts an object for comparison to the error. [[`2d374916eb`](https://github.com/nodejs/node/commit/2d374916eb)]
+  * The error message from `assert.ok(expression)` now also contains the expression itself [[`f76ef50432`](https://github.com/nodejs/node/commit/f76ef50432)]


itself -> itself.?

BridgeAR · 2018-04-26T11:07:19Z

doc/changelogs/CHANGELOG_V10.md

@@ -98,6 +102,7 @@
 * Util
  * `util.types.is[…]` type checks have been added. [[`b20af8088a`](https://github.com/nodejs/node/commit/b20af8088a)]
  * Support for bigint formatting has been added to `util.inspect()`. [[`39dc947409`](https://github.com/nodejs/node/commit/39dc947409)]
+  * Runtime deprecate using a property named `inspect` on an object to specify a custom inspection with `util.inspect()`. [[`617e3e96e6`](https://github.com/nodejs/node/commit/617e3e96e6)]


Hm, I tried to reword it as suggested but it still sounds a bit off to me.

util.inspect() custom inspection with inspect property has been deprecated at runtime.

?

Trott · 2018-04-26T03:53:31Z

doc/changelogs/CHANGELOG_V10.md

@@ -33,15 +33,18 @@

 * Assert
  * Calling `assert.fail()` with more than one argument is deprecated. [[`70dcacd710`](https://github.com/nodejs/node/commit/70dcacd710)]
-  * Calling `assert.ok()` with no arguments will now throw. [[`3cd7977a42`](https://github.com/nodejs/node/commit/3cd7977a42)]


Should this deletion be explained?

@vsemozhetbyt It's incorrect. assert.ok() always threw if there was no argument (probalby because undefined is false-y.) What's changed is that there is a better message in that situation. While a nice change, I suspect it does not warrant calling out in the Notable Changes section.

rvagg · 2018-04-26T03:54:43Z

No opinion on whether it's a good idea to modify the notable changes list after-the-fact like this.
I guess @nodejs/release might have opinions?

I think it should be fine, we've tweaked things like this in the past, it's better to have it accurate and informative after the fact than leave it in a worse state.

jasnell · 2018-04-26T03:56:43Z

I'm generally -1 on it. There's were around 300 major commits and 30 odd minors. It's impractical to list every possible notable change and adding them after the fact doesn't make sense. This is why it's helpful to have more eyes on before the release. The draft notable changes were up for at least a week before.

Trott · 2018-04-26T04:04:43Z

adding them after the fact doesn't make sense.

Why not? As @rvagg points out, "It's better to have it accurate and informative after the fact than leave it in a worse state."

AyushG3112 · 2018-04-26T04:30:05Z

Pardon my ignorance, but everything that landed with the v10.0.0 release should have been part of the 10.0.0 milestone right? Or am I wrong?

BridgeAR · 2018-04-26T10:55:26Z

@AyushG3112 the milestone was added at PRs in the end to distinguish them from other PRs that should land in 11.x. A lot landed in 10.x without that milestone.

BridgeAR · 2018-04-26T11:07:31Z

I addressed the comments.

jasnell · 2018-04-26T16:30:31Z

Why not? As @rvagg points out, "It's better to have it accurate and informative after the fact than leave it in a worse state."

I'm fine with removing an inaccurate item, I'm not fine with adding items. Any and all of the semver-major and semver-minor commits could have made the list and care was taken to break those out into distinct sections in the changelog so that they would be more visible.

BridgeAR · 2018-04-26T16:36:32Z

@jasnell I thought the notable-change label would be enough and the source for these things.

jasnell · 2018-04-26T16:43:01Z

There are 95 issues marked with notable-change at the moment, many from old releases. Going through those manually to reconcile the 750+ commits that landed in this major when there are also CI, CITGM, and other items to go through, is not super practical. What would be good is some improved automation tooling that would find those tags and make them visible. For instance, an update to branch-diff or changelog-maker that included a [NOTABLE CHANGE] marker in the list, similar to how it marks [SEMVER-MAJOR] and [SEMVER-MINOR] commits would make that label significantly more useful.

jasnell · 2018-04-26T16:44:11Z

Also keep in mind that there are many notable changes that are not marked using the notable-change label. As I said, any and all of several hundred semver-major and semver-minor commits could have been considered notable.

BridgeAR · 2018-04-27T12:21:36Z

@rvagg

[...] we've tweaked things like this in the past, it's better to have it accurate and informative after the fact than leave it in a worse state.

I agree, since people will also look at the changelog later on. Especially, when it becomes an LTS release.

@jasnell

It's impractical to list every possible notable change and adding them after the fact doesn't make sense.

I agree that is it difficult to note every possible notable change but I personally think it is actually useful to have this discussion right now to find a way to

There are 95 issues marked with notable-change at the moment, many from old releases.

Yes, but most of them can easily be distinguished because they are often onboarding of a new collaborator and we know that we only have to check the ones that landed since 9.0.0 was released. Older ones can be ignored. That is how I went through the list. I was confused when looking at the notable changes because I expected other entries to be in there. So I decided to go through the ones with that label and that is how I opened the PR.

What would be good is some improved automation tooling that would find those tags and make them visible.

I thought that was already the case. Good to know that it is not.

Also keep in mind that there are many notable changes that are not marked using the notable-change label.

I think we should enforce that. I am going to open a PR to change our guidelines for it.

I think it would still be best to update the list, especially, since I do not see any downside in it. The PR is here and I already did the necessary work.

BridgeAR · 2018-05-14T15:54:39Z

Most people will now have read the changelog even though we will have more people reading the changelog when 10.x becomes the LTS release. I wonder why it is bad to fix these entries since I already did the work @jasnell.

I personally do not see this as something important but I also see no point in not landing this. @nodejs/tsc PTAL and decide what to do here.

TimothyGu

I'm +1 for landing this even after the release of v10.0.0.

TimothyGu · 2018-05-20T15:47:17Z

doc/changelogs/CHANGELOG_V10.md

@@ -52,6 +55,7 @@
  * The `crypto.DEFAULT_ENCODING` property has been deprecated. [[`6035beea93`](https://github.com/nodejs/node/commit/6035beea93)]
  * The `ECDH.convertKey()` method has been added. [[`f2e02883e7`](https://github.com/nodejs/node/commit/f2e02883e7)]
  * The `crypto.fips` property has been deprecated. [[`6e7992e8b8`](https://github.com/nodejs/node/commit/6e7992e8b8)]
+  * The AES-CCM algorithm got implemented. [[`1e07acd476`](https://github.com/nodejs/node/commit/1e07acd476)]


s/got/has been/ to be consistent.

TimothyGu · 2018-05-20T15:50:38Z

doc/changelogs/CHANGELOG_V10.md

@@ -98,6 +102,7 @@
 * Util
  * `util.types.is[…]` type checks have been added. [[`b20af8088a`](https://github.com/nodejs/node/commit/b20af8088a)]
  * Support for bigint formatting has been added to `util.inspect()`. [[`39dc947409`](https://github.com/nodejs/node/commit/39dc947409)]
+  * Runtime deprecate using a property named `inspect` on an object to specify a custom inspection with `util.inspect()`. [[`617e3e96e6`](https://github.com/nodejs/node/commit/617e3e96e6)]


util.inspect() custom inspection with inspect property has been deprecated at runtime.

?

addaleax

LGTM with @TimothyGu’s suggestions

jasnell · 2018-05-21T13:59:11Z

Still not fond of this change but won't block it. Would ask that if this lands, a corresponding update is made to the blog post in the website to ensure that the notable changes shown there are consistent.

jasnell · 2018-05-21T14:00:07Z

In future semver-majors, I would ask that collaborators please give more attention to the proposed notable changes list and changelog before the release is made. It would be helpful.

BridgeAR · 2018-05-21T18:34:21Z

Comments addressed.

New CI https://ci.nodejs.org/job/node-test-pull-request-lite/740/

I will open a PR for the website right after landing this.

mhdawson

LGTM. As long as we agree that these would have been included if identified earlier so we are moving closer to the "right" list then its helpful.

fhinkel

Thanks!

fhinkel · 2018-05-23T22:36:46Z

As discussed in the TSC meeting today, consensus is that this can land. Thanks!

BridgeAR · 2018-05-25T16:32:26Z

Thanks. Landed in cf989b6

A couple entries were missing and one entry was not really relevant. PR-URL: nodejs#20316 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

A couple entries were missing and one entry was not really relevant. PR-URL: #20316 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

doc: update the notable changes

410b268

A couple entries were missing and one entry was not really relevant.

BridgeAR requested review from jasnell, Trott and vsemozhetbyt April 26, 2018 02:31

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 26, 2018

Trott approved these changes Apr 26, 2018

View reviewed changes

vsemozhetbyt approved these changes Apr 26, 2018

View reviewed changes

vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Apr 26, 2018

fixup: address comments

e36e7e2

trivikr approved these changes Apr 26, 2018

View reviewed changes

BridgeAR added the tsc-review label Apr 30, 2018

BridgeAR requested a review from a team April 30, 2018 14:45

BridgeAR added the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label May 14, 2018

TimothyGu reviewed May 20, 2018

View reviewed changes

mhdawson mentioned this pull request May 21, 2018

Node.js Foundation Technical Steering Committee (TSC) Meeting 2018-05-23 nodejs/TSC#544

Closed

addaleax approved these changes May 21, 2018

View reviewed changes

fixup: address comments

95abf9e

TimothyGu approved these changes May 22, 2018

View reviewed changes

mhdawson approved these changes May 23, 2018

View reviewed changes

fhinkel approved these changes May 23, 2018

View reviewed changes

BridgeAR closed this May 25, 2018

BridgeAR mentioned this pull request May 25, 2018

blog: update 10.0.0 notable changes nodejs/nodejs.org#1664

Merged

MylesBorins mentioned this pull request May 29, 2018

v10.3.0 proposal #21011

Merged

BridgeAR deleted the fix-10.x-changelog branch January 20, 2020 11:34

doc: update the notable changes in 10.x #20316

doc: update the notable changes in 10.x #20316

Conversation

BridgeAR commented Apr 26, 2018

Checklist

BridgeAR commented Apr 26, 2018

Trott left a comment

Choose a reason for hiding this comment

Trott commented Apr 26, 2018

vsemozhetbyt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

rvagg commented Apr 26, 2018

jasnell commented Apr 26, 2018

Trott commented Apr 26, 2018

AyushG3112 commented Apr 26, 2018 • edited

BridgeAR commented Apr 26, 2018

BridgeAR commented Apr 26, 2018

jasnell commented Apr 26, 2018

BridgeAR commented Apr 26, 2018

jasnell commented Apr 26, 2018 • edited

jasnell commented Apr 26, 2018

BridgeAR commented Apr 27, 2018

BridgeAR commented May 14, 2018

TimothyGu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax left a comment

Choose a reason for hiding this comment

jasnell commented May 21, 2018

jasnell commented May 21, 2018

BridgeAR commented May 21, 2018

mhdawson left a comment

Choose a reason for hiding this comment

fhinkel left a comment

Choose a reason for hiding this comment

fhinkel commented May 23, 2018

BridgeAR commented May 25, 2018

AyushG3112 commented Apr 26, 2018 •

edited

jasnell commented Apr 26, 2018 •

edited