FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: move topics/guides to website by evanlucas · Pull Request #10896 · nodejs/node · GitHub

/ node Public

doc: move topics/guides to website - #10896

Merged
evanlucas merged 1 commit into
nodejs:masterfrom
evanlucas:rmguides
Jan 24, 2017
ConversationCommits (1)ChecksFiles changed
Merged

doc: move topics/guides to website#10896
evanlucas merged 1 commit into
nodejs:masterfrom
evanlucas:rmguides

Conversation

evanlucas commented Jan 19, 2017

Copy link
Copy Markdown
Contributor

This commit removes the topics and guides that the documentation
working group has proposed added to the website. We want them to have
more visibility and believe that moving them to the website does that.

This depends on nodejs/nodejs.org#1105 landing.

/cc @nodejs/documentation

Checklist
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

nodejs-github-bot added doc Issues and PRs related to the documentations. lts-watch-v6.x labels Jan 19, 2017

gibfahn commented Jan 19, 2017

Copy link
Copy Markdown
Member

Maybe Fixes: https://github.com/nodejs/node/issues/10792 in the commit message?

evanlucas commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

@gibfahn good call. Thanks!

gibfahn commented Jan 19, 2017

Copy link
Copy Markdown
Member

So looking at the line counts between this and nodejs/nodejs.org#1105, it looks like:

File Move Old line count New line count
timers-in-node.md straight move -192 +192
blocking-vs-non-blocking.md extra lines -143 +148
domain-postmortem.md extra lines -301 +444
event-loop-timers-and-nexttick.md extra lines -486 +492
domain-resource-cleanup-example.js gone -136

Did domain-resource-cleanup-example.js get merged into something else?

evanlucas commented Jan 19, 2017

Copy link
Copy Markdown
Contributor Author

Yes, I had to inline the js file.

gibfahn approved these changes Jan 19, 2017
View reviewed changes

gibfahn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, the diffs match up

lpinca approved these changes Jan 19, 2017
View reviewed changes

lpinca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Rubber-stamp LGMT

sam-github approved these changes Jan 19, 2017
View reviewed changes

sam-github left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, Evan.

doc: move topics/guides to website
7eef09d 
This commit removes the topics and guides that the documentation
working group has proposed added to the website. We want them to have
more visibility and believe that moving them to the website does that.

Ref: nodejs/nodejs.org#1105
Fixes: nodejs#10792
PR-URL: nodejs#10896
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
evanlucas closed this Jan 24, 2017
evanlucas deleted the rmguides branch January 24, 2017 13:42

evanlucas commented Jan 24, 2017

Copy link
Copy Markdown
Contributor Author

Landed in 7eef09d. Thanks!

evanlucas merged commit 7eef09d into nodejs:master Jan 24, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
doc: move topics/guides to website
8e6586e 
This commit removes the topics and guides that the documentation
working group has proposed added to the website. We want them to have
more visibility and believe that moving them to the website does that.

Ref: nodejs/nodejs.org#1105
Fixes: nodejs#10792
PR-URL: nodejs#10896
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas mentioned this pull request Jan 25, 2017
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
doc: move topics/guides to website
3177d65 
This commit removes the topics and guides that the documentation
working group has proposed added to the website. We want them to have
more visibility and believe that moving them to the website does that.

Ref: nodejs/nodejs.org#1105
Fixes: nodejs#10792
PR-URL: nodejs#10896
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas mentioned this pull request Jan 29, 2017
Merged

MylesBorins commented Mar 7, 2017

Copy link
Copy Markdown
Contributor

So I'm not sure what we want to do about this for LTS... It just dawned on me that in the past our guides were versioned... so the proper guide was with LTS.

Now we may have the wrong guides...

/cc @nodejs/lts

MylesBorins commented Apr 13, 2017

Copy link
Copy Markdown
Contributor

ping @nodejs/lts again

sam-github commented Apr 14, 2017

Copy link
Copy Markdown
Contributor

Only the API docs from LTS branches shows up on the website, so there isn't much reason to backport this, is there? Since we won't be cherry picking anything back that touches the guides, since they don't exist on the master, it won't even help avoid future conflicts.

gibfahn commented Apr 16, 2017

Copy link
Copy Markdown
Member

So I'm not sure what we want to do about this for LTS... It just dawned on me that in the past our guides were versioned... so the proper guide was with LTS.

Now we may have the wrong guides...

When this was previously discussed in the docs WG, the general feeling was that we should do this first to get the guides more visible, and worry about things like multiple versions later. The alternative to this would be to do what we do with the api docs, but that's a lot more involved. Previously, no-one really knew that the user guides existed, so this is at least a step in the right direction.

MylesBorins commented May 8, 2017

Copy link
Copy Markdown
Contributor

@gibfahn based on that, should we backport the removal?

sam-github commented May 8, 2017

Copy link
Copy Markdown
Contributor

@MylesBorins I can't find any signs that the guides even show up on the website for older releases, am I missing it?

Given that, back-port or not is just a matter of what makes maintaing LTS easier. Since its just removing files, I don't think we need this, there won't be conflicts.

gibfahn commented May 8, 2017

Copy link
Copy Markdown
Member

I can't find any signs that the guides even show up on the website for older releases, am I missing it?

@sam-github The new location is directly at https://nodejs.org/en/docs/guides/, so they're not tied to a release. The old guides weren't on the website at all, they just sat in the source tree.

based on that, should we backport the removal?

Either way is fine. Shouldn't be any problem to backport, but it shouldn't cause any problems if we don't. So let's say no.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Qard Qard approved these changes
jasnell jasnell approved these changes
lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
+3 more reviewers
sam-github sam-github approved these changes
thefourtheye thefourtheye approved these changes
gibfahn gibfahn approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

Back | FazBrowse Home | New Git URL