-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
blog: add release post for 8.0.0 #1241
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
Conversation
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: noticeable and significant
-> significant
; application performance
-> performance
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Remove To mitigate the loss in performance
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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-inspect
has 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove still considered
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Remove As an even numbered release,
Nit: represents
-> is
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
October
-> October 2017
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we've decided to simply drop
-> we've dropped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nit: unnecessary space before semicolon.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong quotes, parsing error.
locale/en/blog/release/v8.0.0.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Wrong quotes, parsing error.
- Maybe
(message) => { /** ... **/ }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also funny quotes on require(‘inspector’)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems they are also fixed)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@cjihrig @mcollina @nodejs/ctc @ZibbyKeaton ... here's the draft. PTAL