Skip to content

Conversation

@wesleytodd
Copy link
Member

If this in fact works then it is a nice addition since it is a bit more complex than the existing examples in the readme. @whitlockjc Can you update if the solution actually solves your problem?

@whitlockjc
Copy link

If I understand this example, I would need to use .all for each path. Is that correct?

@wesleytodd
Copy link
Member Author

wesleytodd commented Nov 22, 2017

Yes, that is correct. I would say you might want to write a little helper to wrap it up:

var route = router.route('/foo')
// add methods...
handle405(route)

@whitlockjc
Copy link

Yep, that's what I'd do. Let me give it a shot.

@wesleytodd
Copy link
Member Author

wesleytodd commented Nov 29, 2017

Anyone have input on adding examples like this? @whitlockjc said that the approach worked for his use case. If no one opposed it would be nice to get this merged.

@whitlockjc
Copy link

I wish there was a better way to do this, like a single .all for all routes. But this does work as-is, verified.

@dougwilson dougwilson self-assigned this Jul 2, 2018
dougwilson pushed a commit to wesleytodd/router that referenced this pull request Jul 2, 2018
Copy link
Contributor

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

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

So I made some modifications, mainly just some rewording and fixing the example to not use Express-only syntax like res.status() which wouldn't work if the example was actually ran.

@dougwilson dougwilson merged commit 7c2ee89 into pillarjs:master Jul 2, 2018
@wesleytodd
Copy link
Member Author

not use Express-only syntax

Good call, I will remember this for the future.

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.

3 participants