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

Separating Atmosphere and npm package sections #410

Merged
merged 45 commits into from May 5, 2016

Conversation

Projects
None yet
5 participants
@jamielob
Contributor

jamielob commented Apr 29, 2016

Hey @tmeasday, here's the PR we discussed.

Changed Build Section organization to separate Atmosphere and npm.

discourseTopicId: 20192
---
Atmosphere packages are packages written specifically for Meteor and have several advantages over npm when used with Meteor. In particular, Atmosphere packages can:

This comment has been minimized.

@stubailo

stubailo Apr 30, 2016

Contributor

I'd move this to the atmosphere vs npm section, since if you are comparing the two you would want this information.

This comment has been minimized.

@stubailo

stubailo Apr 30, 2016

Contributor

(Same for the analogous section in the NPM article?)

This comment has been minimized.

@jamielob

jamielob Apr 30, 2016

Contributor

I want to keep things in line with the rest of the guide - is it bad practice to have a <h2> as the first thing in a section? i.e. Should there always be some introductory text?

discourseTopicId: 20194
---
**NOTE:** If you want to override a package that already exists in the npm registry, [use this method](#overriding-npm-packages).

This comment has been minimized.

@stubailo

stubailo Apr 30, 2016

Contributor

I wonder why this is the first thing in the article - if it is in the table of contents I suspect people will find it!

This comment has been minimized.

@jamielob

jamielob Apr 30, 2016

Contributor

I think it's left over from when it wasn't organized this way. I'll remove it.

@stubailo

This comment has been minimized.

Contributor

stubailo commented Apr 30, 2016

Also, something to think about is whether we should redirect the old page URLs somehow, but I'm not sure exactly how that would work.

jamielob added some commits Apr 30, 2016

Added npm hexo-generator-alias for redirects
Couldn't find a way to redirect specific # entry points, but I have
redirected the old using-packages and writing-packages pages to the new
atmosphere-vs-npm page.
@jamielob

This comment has been minimized.

Contributor

jamielob commented May 2, 2016

Hey @stubailo - I've added redirects using hexo-generator-alias. I couldn't find an easy way with hexo to redirect specific # entry points so I've redirected both the old pages to the new atmosphere-vs-npm page so that any old links don't get re-directed to an incorrect atmosphere or npm specific topic.

@tmeasday

This comment has been minimized.

Contributor

tmeasday commented May 2, 2016

@jamielob this looks great! Thanks so much!

I think we need a better system for the redirects however. I've already had to do something similar for the docs so let me think a bit about how to make it generic and use it here.

@stubailo

This comment has been minimized.

Contributor

stubailo commented May 2, 2016

@tmeasday does S3 support a 404 route? Then we could just do a JS redirect from there or something?

@tmeasday

This comment has been minimized.

Contributor

tmeasday commented May 2, 2016

Yes, it sure does. We'll make 404s -> index and then the index figure it out.

But the tricky part is outlining the hashes .. like

using-packages.html#client-npm -> using-npm-packages.html#client-npm
@jamielob

This comment has been minimized.

Contributor

jamielob commented May 2, 2016

I've just gone through the old using-packages section and mapped each of the id entry points to their new locations. If this is helpful for you, let me know and I'll do the same for the writing-packages page.

using-packages.md#npm > using-npm-packages.md
using-packages.md#client-npm > using-npm-packages.md#client-npm
using-packages.md#installing-npm > using-npm-packages.md#installing-npm
using-packages.md#using-npm > using-npm-packages.md#using-npm
using-packages.md#npm-styles > using-npm-packages.md#npm-styles
using-packages.md#npm-shrinkwrap > using-npm-packages.md#npm-shrinkwrap 
using-packages.md#atmosphere >  using-atmosphere-packages.md
using-packages.md#atmosphere-searching > using-atmosphere-packages.md#atmosphere-searching
using-packages.md#atmosphere-naming > using-atmosphere-packages.md#atmosphere-naming
using-packages.md#installing-atmosphere > using-atmosphere-packages.md#installing-atmosphere
using-packages.md#using-atmosphere > using-atmosphere-packages.md#using-atmosphere
using-packages.md#importing-atmosphere-styles > using-atmosphere-packages.md#importing-atmosphere-styles
using-packages.md#peer-npm-dependencies > using-atmosphere-packages.md#peer-npm-dependencies
using-packages.md#package-namespacing > using-atmosphere-packages.md#package-namespacing
using-packages.md#async-callbacks > using-npm-packages.md#async-callbacks
using-packages.md#bind-environment > using-npm-packages.md#bind-environment
using-packages.md#wrap-async > using-npm-packages.md#wrap-async
using-packages.md#promises > using-npm-packages.md#promises
using-packages.md#overriding-packages >  writing-npm-packages.md#overriding-npm-packages
using-packages.md#npm-overriding > writing-npm-packages.md#overriding-npm-packages
using-packages.md#atmosphere-overriding > writing-atmosphere-packages.md#overriding-atmosphere-packages
using-packages.md#npm-shrinkpack > using-npm-packages.md#npm-shrinkpack
@tmeasday

This comment has been minimized.

Contributor

tmeasday commented May 2, 2016

@jamielob - thanks. Sure, if it's easy--otherwise I can do it pretty easily too.

@jamielob

This comment has been minimized.

Contributor

jamielob commented May 5, 2016

Well that was a 🐶

@tmeasday Mind if I leave the redirects for you? I removed the hexo alias package and basic forwarding that I had originally done.

@tmeasday

This comment has been minimized.

Contributor

tmeasday commented May 5, 2016

No problem @jamielob. Let's get this merged ASAP :)

Thanks for your hard work on this.

@tmeasday tmeasday merged commit 87c96b5 into meteor:master May 5, 2016

2 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
@jamielob

This comment has been minimized.

Contributor

jamielob commented May 6, 2016

Cheers! 👍

tmeasday added a commit that referenced this pull request May 6, 2016

@tmeasday

This comment has been minimized.

Contributor

tmeasday commented May 17, 2016

Thanks @PascalPencil -- the redirects should work[1] - I must have broken them.

[1] It does actually link to guide.meteor.com

@Superpencil

This comment has been minimized.

Superpencil commented May 18, 2016

@tmeasday keep up the good work 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment