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

Middleware can be inserted before or after defaults #1393

Merged
merged 1 commit into from
May 12, 2016

Conversation

ridiculous
Copy link
Contributor

Builds onto the new middleware stack introduced in #1390. This is useful because it allows positioning of middleware around default Grape middleware.

@ridiculous ridiculous force-pushed the lazy-middleware-stack branch 2 times, most recently from a72b8ec to 24024d2 Compare May 12, 2016 04:01
@ridiculous ridiculous changed the title Middleware can be inserted before or after default middlewares Middleware can be inserted before or after defaults May 12, 2016
@@ -5,6 +5,7 @@

#### Features

* [#1393](https://github.com/ruby-grape/grape/pull/1393): Middleware can be inserted before or after default Grape middleware - [@ridiculous](https://github.com/ridiculous).
Copy link
Member

Choose a reason for hiding this comment

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

I think the CHANGELOG entry below covers this functionality, I would just add , [#1393] to it. No big deal.

@dblock dblock merged commit 7e6daeb into ruby-grape:master May 12, 2016
@dblock
Copy link
Member

dblock commented May 12, 2016

Very nice, merged.

@rosa
Copy link
Contributor

rosa commented May 16, 2016

Nice :) Although with only #1390 you could position your middleware around the default middleware, for example with:

insert_before Grape::Middleware::Error, MyMiddleware

Just out of curiosity, what was the use case not considered?

@dblock
Copy link
Member

dblock commented May 16, 2016

Sorry @rosa I think I don't understand the question :)

@rosa
Copy link
Contributor

rosa commented May 16, 2016

@dblock sorry for not explaining myself better! 😅 I meant to ask about which new feature this PR introduces that wasn't already covered by #1390. I believed you could already position middleware around defaults, so I was wondering if I had missed some use case that is now covered.

@ridiculous
Copy link
Contributor Author

ridiculous commented May 16, 2016

@rosa It let's you reference middleware added after https://github.com/ruby-grape/grape/pull/1393/files#diff-8e15cddb80f42ca7d43872846d80ecbeR263, which is pretty much just the formatter and versioner

@rosa
Copy link
Contributor

rosa commented May 16, 2016

Ah, got it! I was using my changes just to insert a middleware before Grape::Middleware::Error, which was fine but as you noticed, it didn't cover all cases. Thanks for the clarification! 😄

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