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

Added chainable route handlers for some routes. #146

Closed
wants to merge 1 commit into from
Closed

Added chainable route handlers for some routes. #146

wants to merge 1 commit into from

Conversation

SamuelPinho
Copy link
Contributor

@SamuelPinho SamuelPinho commented Sep 26, 2017

Some paths can be specified in just one line.


This change is Reviewable

@robertcoopercode
Copy link
Collaborator

Some tests are failing, so I'll clone your branch @SamuelPinho and have a look at what's wrong. I like the idea of making the code more concise with chainable route handlers!

@robertcoopercode
Copy link
Collaborator

Oh, I see what the issue is @SamuelPinho. We are currently running Express 3.x and we haven't yet migrated the code to use Expresx 4.x, so we cannot implement chainable routes at the moment. If you end up refactoring the project to use Express 4.x, that would be extremely helpful!

@SamuelPinho
Copy link
Contributor Author

Perfect @Engineering-Robert thank you.
Actually I'm trying to organize routes in one folder file by file and then load them through express-load. After that I think the refactoring is going to be more easily done.

@robertcoopercode
Copy link
Collaborator

That's awesome! Thanks a lot @SamuelPinho. Looking forward to your next PR!

@vinitkumar
Copy link
Owner

@SamuelPinho Have you already ported to the code to express-4 in your fork?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants