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

Fix rails middleware list in api_app guide [ci skip] #30668

Merged
merged 1 commit into from Sep 24, 2017

Conversation

yhirano55
Copy link
Contributor

@yhirano55 yhirano55 commented Sep 20, 2017

Summary

It seems that middleware list on api-mode is different from actual result:

$ bin/rails middleware
use Rack::Sendfile
use ActionDispatch::Static
use ActionDispatch::Executor
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use ActionDispatch::RequestId
use ActionDispatch::RemoteIp
use Rails::Rack::Logger
use ActionDispatch::ShowExceptions
use ActionDispatch::DebugExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
run MyApi::Application.routes
  • MyApi::Application::Routes is a rack application, not a middleware. So I removed it.

@rails-bot
Copy link

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

@kamipo
Copy link
Member

kamipo commented Sep 21, 2017

MyApi::Application.routes is an app, not a middleware. It should be removed in the list I think.

@yhirano55
Copy link
Contributor Author

Oh… you're right. I'll remove it.

* `MyApi::Application::Routes` is not middleware.
@yhirano55
Copy link
Contributor Author

yhirano55 commented Sep 23, 2017

@kamipo I removed it. please review it again.

@kamipo kamipo merged commit 6aa5646 into rails:master Sep 24, 2017
@yhirano55 yhirano55 deleted the fix_api_guide branch September 24, 2017 11:08
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

4 participants