Conversation
jasnell
changed the title
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: Simplify:
This article contains a summary of the most significant changes and features.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: noticeable and significant -> significant; application performance -> performance
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: Remove To mitigate the loss in performance
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
At a minimum: considered to be unsupported -> deprecated
But really, I'd say remove this sentence altogether. We can't truly guarantee that it will not be removed from Node.js ever (although I agree it's unlikely).
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: remove comma after time (but leave the one after environments)
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
an fully supported -> a fully supported
I think this paragraph is a fair bit too wordy. Something succinct might be better. Maybe drop a couple sentences so it looks like this?:
The legacy command line debugger is being removed in Node.js 8. As a command
line replacement,node-inspecthas been integrated directly into the Node.js
runtime. Additionally, the V8 Inspector debugger, which arrived previously as
an experimental feature in Node.js 6, is being upgraded to a fully supported
feature.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: Remove considered. (It's either experimental or it's not. Rather than provide clarity, 'considered' just muddies things.)
locale/en/blog/release/v8.0.0.md
Outdated
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: Remove As an even numbered release,
Nit: represents -> is
locale/en/blog/release/v8.0.0.md
Outdated
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: Remove once the Node.js 8 release line has had several months to completely stabilize
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Nit: Change first sentence in this paragraph to Note that we have dropped the "v" in Node.js 8.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
we've decided to simply drop -> we've dropped
Trott
left a comment
Trott
left a comment
There was a problem hiding this comment.
Left a bunch of nits, but overall: Wow, this is a lot to summarize and you've organized it well. 👍
|
Updated to address the comments. Will update again once the release is out. |
|
/cc @nodejs/documentation |
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
A nit: unnecessary space before semicolon.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Wrong quotes, parsing error.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
- Wrong quotes, parsing error.
- Maybe
(message) => { /** ... **/ }?
There was a problem hiding this comment.
also funny quotes on require(‘inspector’)
There was a problem hiding this comment.
It seems they are also fixed)
jasnell
changed the title
|
Landed with fixes made. Thank you all |
|
@jasnell I think you only pushed on your fork. |
|
I need a beer. fixing |
|
Sorry, is this OK:
Should this be |
to redirected -> be redirected ? |
These are links for the beta, maybe they should be the nodejs/node@c58cea5 and nodejs/node#13276 instead. |
Wrong link formatting, wrong (not shortened) rendering in the https://nodejs.org/en/blog/release/v8.0.0/ See also nodejs/node#13313 |
|
Maybe it is too late for this, but should not we mention in notable changes some |
Procedure might be to open a pull request to make the changes. /cc @nodejs/website in case I'm wrong about that... |
|
@vsemozhetbyt ... no problems :-) we can make edits to the blog post. Just add commits to the PR I see you've already opened :-) |
|
|
||
| Zero-filling new instances of `Buffer(num)` by default will have a significant | ||
| impact on performance. Developers should move to the new | ||
| `Buffer.allocUnsafe(num)` API if they wish to allocate `Buffer` instances |
There was a problem hiding this comment.
This probably should have also mentioned Buffer.alloc (for consistent behaviour between versions), and an additional warning against using uninitialized memory allocations.
@jasnell, would it be acceptable to update the release post now? If yes, I will be willing to propose a small patch.
10 participants
@cjihrig @mcollina @nodejs/ctc @ZibbyKeaton ... here's the draft. PTAL