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

Convert middleware once instead of doing it during routing #14

Merged
merged 1 commit into from
Jan 20, 2017

Conversation

DoumanAsh
Copy link
Contributor

@DoumanAsh DoumanAsh commented Jan 20, 2017

I'm not sure if there is reasoning for that...
But as of now alignment of different kinds of middlewares is done upon routing.
I believe it might be fairly ineffective to do it all the time, especially if number of middlewares is high.

Since route can be created only at .createRoute method i moved it there.
But i'm not sure if user is allowed to modify middlewares of route.
If it is allowed then, i suppose conversion is required at routing time, still it is fairly bad from my point of view.

Tests are passed locally.

@coveralls
Copy link

coveralls commented Jan 20, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 763aab9 on DoumanAsh:middle_convert_once into d0c2887 on tunnckoCore:master.

@tunnckoCore
Copy link
Owner

tunnckoCore commented Jan 20, 2017

Yea, good point. Thanks!

Next time, please use commit convention message.

@tunnckoCore tunnckoCore merged commit 1ce2d4c into tunnckoCore:master Jan 20, 2017
@DoumanAsh
Copy link
Contributor Author

Next time, please use commit convention message.

Sorry, i think i missed it... where can i find guideline you use for commit messages?

@tunnckoCore
Copy link
Owner

Hm. My fault. I just realized that I didn't update the README to my new style.

Can be found here for now https://github.com/tunnckoCore/resolve-plugins-sync#contributing and a i'll just update it here :) Just use npm run commit instead of git commit. :)

tunnckoCore pushed a commit that referenced this pull request Jan 20, 2017
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.

None yet

3 participants