Navigation Menu

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

Get rid of the default_filters method #1781

Merged
merged 1 commit into from Oct 1, 2014
Merged

Conversation

namusyaka
Copy link
Contributor

The default_filters method seems no need to us.
Also the filter has continued to break the compatibility with Sinatra.
Thoughts?

@ujifgc
Copy link
Member

ujifgc commented Oct 1, 2014

👍

1 similar comment
@tyabe
Copy link
Contributor

tyabe commented Oct 1, 2014

👍

@@ -89,7 +89,7 @@
@app.reload!
get "/rand"
refute_equal last_body, body
assert_equal 2, @app.filters[:before].size # one is ours the other is default_filter for content type
assert_equal 1, @app.filters[:before].size # one is ours the other is default_filter for content type

Choose a reason for hiding this comment

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

we should update this comment and the one above on line 82?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks!

Fixes wrong value of content-type when using css template helpers
The :default_content_type should be respected for compatibility with
Sinatra
@dariocravero
Copy link

👍

@namusyaka namusyaka added this to the 0.12.4 milestone Oct 1, 2014
@namusyaka
Copy link
Contributor Author

Thank you, everyone 👍

namusyaka added a commit that referenced this pull request Oct 1, 2014
Get rid of the default_filters method
@namusyaka namusyaka merged commit fe65b1a into master Oct 1, 2014
@namusyaka namusyaka deleted the sinatra-compatibility branch October 1, 2014 10:54
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