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

readme: add docs #3

Closed
wants to merge 1 commit into from
Closed

readme: add docs #3

wants to merge 1 commit into from

Conversation

Fishrock123
Copy link
Member

Continued from #2

cc @rlidwka

@@ -8,42 +8,280 @@

Simple middleware-style router

## Installation
### Installation
Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot establish a h3 when no h2 even exists; please make these make sense from a hierarchy instead of how GitHub happens to make them look.

Copy link
Member Author

Choose a reason for hiding this comment

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

Quite frankly I care much more about ease of reading.

Copy link
Contributor

Choose a reason for hiding this comment

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

WTF? The difference is between 1.75em and 1.5em and would allow us to use a TOC generator if it's kept right.

Copy link
Member Author

Choose a reason for hiding this comment

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

I work in front-end and with design everyday, these things bug me. :P

I don't understand why this would be in the TOC, or where we would put a full(?) TOC?

Besides this is regular html stuff, shouldn't a generator be able to handle this?

Copy link
Contributor

Choose a reason for hiding this comment

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

GitHub may be adding an automatic TOC soon. It doesn't skip anything. I have no idea how it will work if you skip levels, perhaps by precluding us the ability to use it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Bu yea, if you want it to be level 3, we need to move it under an existing level 2, or maybe just remove the installation section all together, since we don't think it fits in it's current location by changing the level.

Copy link
Member Author

Choose a reason for hiding this comment

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

Skipping levels isn't uncommon. iirc there's nothing unsemantic about it in markdown.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just remove the section, move it, or put it back to level 2 is all I'm saying.

Markdown spec at https://daringfireball.net/projects/markdown/basics makes it 100% clear that directly map to HTML header levels and the HTML spec http://www.w3.org/TR/html401/struct/global.html#h-7.5.5 mentions that people believe skipping is a bad practice, as it should be, since HTML does not represent styling, but document structure.

@Fishrock123
Copy link
Member Author

Note to self: this does not cover error handling middleware.

Fishrock123 added a commit that referenced this pull request Nov 19, 2014
@Fishrock123 Fishrock123 self-assigned this Nov 20, 2014
@Fishrock123
Copy link
Member Author

@dougwilson unless you have other comments I'd like to merge this tonight and then open issues for the other two things.

@Fishrock123
Copy link
Member Author

@dougwilson ok landing if LGTY


var router = new Router()
The [http methods](https://github.com/visionmedia/node-methods/blob/master/index.js) provide the routing functionality in `router`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this link for the new org name.

@Fishrock123
Copy link
Member Author

@dougwilson fixed up most of the things, I think. Except the weird wording on the description.

@jonathanong
Copy link
Member

@Fishrock123 nice!!!

@jonathanong jonathanong reopened this Nov 24, 2014
- [.route()](https://github.com/pillarjs/router#routerroutepath)
- [__Route()__](https://github.com/pillarjs/router#routerroutepath-1)
- [`.{http method}()](https://github.com/pillarjs/router#routehttp-methodhandler) `.get() / .post() / etc`
- [.all()](https://github.com/pillarjs/router#routeallhandler)
Copy link
Contributor

Choose a reason for hiding this comment

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

hm. I didn't realize these where links to GitHub; they don't work on the GItHub mobile website.

@dougwilson dougwilson deleted the readme-docs branch December 22, 2014 00:09
Fishrock123 added a commit that referenced this pull request Dec 22, 2014
Fishrock123 added a commit that referenced this pull request Dec 22, 2014
Fishrock123 added a commit that referenced this pull request Dec 22, 2014
Fishrock123 added a commit that referenced this pull request Dec 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants