doc: update and modernize examples in api docs by jasnell · Pull Request #4282 · nodejs/node · GitHub

Use single quotes consistently
Modernize examples to use template strings and arrow funcs
Fix typos
Fix a few line wrap issues

/cc @nodejs/http

should we do a pass at replacing all instances of require in docs with const?

Incrementally, perhaps. I don't think it's critical enough to do all at once

fair enough. my only thought on this is that it might be weird to have things in the docs inconsistent, but it makes sense to avoid massive churn

It might actually be better to have huge docs only churn in a single commit, instead of 32 commits. People are a lot less likely to bisect and blame the docs.

Looking it over, I agree. It's not a huge chunk of work. Already have it
mostly done. Will update this PR with the other updates.
On Dec 14, 2015 4:51 PM, "Colin Ihrig" notifications@github.com wrote:

In doc/api/http.markdown
#4282 (comment):
@@ -103,7 +103,7 @@ of these values set to their respective defaults.
To configure any of them, you must create your own [http.Agent][] object.
-var http = require('http');
+const http = require('http');
It might actually be better to have huge docs only churn in a single
commit, instead of 32 commits #3662.
People are a lot less likely to bisect and blame the docs.

—
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/4282/files#r47583137.

LGTM. Small comment regarding using const in docs above, but that is not relevant to this landing

Argh, the ES6 inline functions continue to creep in!

lol ... they're not the prettiest thing in the world but it's the direction things are heading

I don't think we have any linting rules for arrow functions yet, but from a quick code search, it looks like the convention has been to put spaces on each side of the arrow. So, in this case (res) => {.

agreed, I'm only seeing (args) => { in use in the wild at the moment with the extra space (not that I'm looking very widely)

In this case it's probably fine to omit the parens, as well — http.get(options, res => {

my vote would be to never omit parens to be explicit what's going on, I personally find the paren-free version little too terse

It's definitely a brand new 🚲🏡; I don't feel strongly enough either way to propose setting a hard-and-fast rule for it. In my own projects, I lean towards minimizing characters in code examples to give other info more room, but two characters is not a huge amount of savings so I'm comfortable leaving this as a personal preference of the doc author for the time being.

@thealphanerd @cjihrig ... ok, just pushed a big update. Fixed up all of the doc examples at once.

@nodejs/documentation

Are you comfortable asserting that the code examples still work as authored? If so, this LGTM pending arrow function style nits!

I'm going to be going through and testing the various cases. Gimme a day to
get through them. Pretty certain everything is good tho
On Dec 14, 2015 9:44 PM, "Chris Dickinson" notifications@github.com wrote:

Are you comfortable asserting that the code examples still work as
authored? If so, this LGTM pending arrow function style nits!

—
Reply to this email directly or view it on GitHub
#4282 (comment).

@jasnell Awesome, thanks — great work!

While we're at it, is there a good reason to keep semicolons in these?

LGTM

what change happened here?

Not a clue. Likely a whitespace change. Will fix.

New changes look good. Left one comment directly on the commit instead of the PR.

const can be used without 'use strict', it's let that can't be used without it.

bash-3.2$ cat ~/tmp/test.js
const m = 1;
let n = 1;
bash-3.2$ node ~/tmp.test
module.js:339
    throw err;
    ^

Error: Cannot find module '/Users/james/tmp.test'
    at Function.Module._resolveFilename (module.js:337:15)
    at Function.Module._load (module.js:287:25)
    at Function.Module.runMain (module.js:467:10)
    at startup (node.js:134:18)
    at node.js:961:3
bash-3.2$

Uh, yeah, never mind my previous comments about strict mode...

Pushed update to pull the "fix" for #4296 back out. That change isn't necessary for master or v5

@cjihrig ... updated the 0022 to 0o022 per your suggestion

I marked this for LTS watch but it likely won't land cleanly. If/when it lands, I'll put together a modified version for v4.x-staging

I'd like to get this landed soon but would appreciate a bit more eyes on it. /cc @nodejs/documentation

Rebased and updated...

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos

Additional example edits for consistency

Per @cjihrig's suggestion

Getting this landed momentarily

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos * Example edits for consistency PR-URL: #4282 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Landed in 9b21119

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos * Example edits for consistency PR-URL: nodejs#4282 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos * Example edits for consistency PR-URL: #4282 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos * Example edits for consistency PR-URL: nodejs#4282 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos * Example edits for consistency PR-URL: #4282 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos * Example edits for consistency PR-URL: nodejs#4282 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

jasnell added http Issues or PRs related to the http subsystem. doc Issues and PRs related to the documentations. labels Dec 14, 2015

MylesBorins reviewed Dec 14, 2015
View reviewed changes

cjihrig reviewed Dec 15, 2015
View reviewed changes

jasnell force-pushed the fix-http-doc branch from b1aaf1e to 49414c1 Compare December 15, 2015 03:38

jasnell changed the title doc: update examples in http.markdown doc: update and modernize examples in api docs Dec 15, 2015

MylesBorins reviewed Dec 15, 2015
View reviewed changes

jasnell added the lts-watch-v4.x label Dec 15, 2015

jasnell force-pushed the fix-http-doc branch from 184623b to 6912a17 Compare December 16, 2015 03:03

jasnell force-pushed the fix-http-doc branch from 0961cf3 to 866efa0 Compare December 16, 2015 16:45

jasnell added 3 commits December 17, 2015 08:02

doc: fix, modernize examples in docs

f950904

* Use single quotes consistently * Modernize examples to use template strings and arrow funcs * Fix a few typos

doc: additional example edits

bae6728

Additional example edits for consistency

doc: update 0022 to 0o022 in process.markdown

aab18d0

Per @cjihrig's suggestion

jasnell force-pushed the fix-http-doc branch from 866efa0 to aab18d0 Compare December 17, 2015 16:03

jasnell closed this Dec 17, 2015

jasnell mentioned this pull request Dec 17, 2015

Release 4.2.4 Planning #4321

Closed

MylesBorins added land-on-v4.x and removed lts-watch-v4.x labels Dec 30, 2015

MylesBorins mentioned this pull request Jan 19, 2016

V4.2.5 proposal #4768

Merged

ChALkeR mentioned this pull request Jan 24, 2016

doc: refactor function expressions to arrow functions #4832

Closed

doc: update and modernize examples in api docs by jasnell · Pull Request #4282 · nodejs/node · GitHub

Uh oh!

Uh oh!

doc: update and modernize examples in api docs - #4282

doc: update and modernize examples in api docs#4282jasnell wants to merge 3 commits intonodejs:masternodejs/node:masterfrom jasnell:fix-http-docjasnell/node:fix-http-docCopy head branch name to clipboard

Conversation

jasnell commented Dec 14, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MylesBorins commented Dec 14, 2015

Uh oh!

mscdex commented Dec 14, 2015

Uh oh!

jasnell commented Dec 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell commented Dec 15, 2015

Uh oh!

jasnell commented Dec 15, 2015

Uh oh!

chrisdickinson commented Dec 15, 2015

Uh oh!

jasnell commented Dec 15, 2015

Uh oh!

chrisdickinson commented Dec 15, 2015

Uh oh!

Fishrock123 commented Dec 15, 2015

Uh oh!

cjihrig commented Dec 15, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjihrig commented Dec 15, 2015

Uh oh!

jasnell commented Dec 16, 2015

Uh oh!

Trott commented Dec 16, 2015

Uh oh!

jasnell commented Dec 16, 2015

Uh oh!

jasnell commented Dec 16, 2015

Uh oh!

jasnell commented Dec 16, 2015

Uh oh!

jasnell commented Dec 16, 2015

Uh oh!

jasnell commented Dec 16, 2015

Uh oh!

jasnell commented Dec 17, 2015

Uh oh!

jasnell commented Dec 17, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

doc: update and modernize examples in api docs#4282
jasnell wants to merge 3 commits into
nodejs:masterfrom
jasnell:fix-http-doc