Skip to content
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

doc: move topics/guides to website #10896

Merged
merged 1 commit into from
Jan 24, 2017
Merged

Conversation

evanlucas
Copy link
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 nodejs-github-bot added doc Issues and PRs related to the documentations. lts-watch-v6.x labels Jan 19, 2017
@gibfahn
Copy link
Member

gibfahn commented Jan 19, 2017

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

@evanlucas
Copy link
Contributor Author

@gibfahn good call. Thanks!

@gibfahn
Copy link
Member

gibfahn commented Jan 19, 2017

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
Copy link
Contributor Author

Yes, I had to inline the js file.

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

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

LGTM, the diffs match up

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

Rubber-stamp LGMT

Copy link
Contributor

@sam-github sam-github left a comment

Choose a reason for hiding this comment

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

Thanks, Evan.

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
Copy link
Contributor Author

Landed in 7eef09d. Thanks!

@evanlucas 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
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 pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
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 italoacasas mentioned this pull request Jan 29, 2017
@MylesBorins
Copy link
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
Copy link
Contributor

ping @nodejs/lts again

@sam-github
Copy link
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
Copy link
Member

gibfahn commented Apr 16, 2017

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
Copy link
Contributor

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

@sam-github
Copy link
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
Copy link
Member

gibfahn commented May 8, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants