Skip to content

Support more kindly for after method #1243

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

namusyaka
Copy link
Contributor

ref #1240

The current implementation is going to try to return after_response if after_response is not false.
I guess the behavior is not expected, and users will be confusing.
Thoughts?

@dblock
Copy link
Member

dblock commented Jan 13, 2016

I think we would be trying to do too much for the user here. There should be a contract since this is an API. Right now it says you need to return a Rack compatible response from after, and I think we shouldn't be having code at runtime trying to "help" API authors work around that.

I vote to close this without merging, lets leave it here for a bit see if others think strongly otherwise.

@namusyaka
Copy link
Contributor Author

Now, we must avoid returning value except false or nil on filters. I guess the constraint is very confusing. Thoughts?

@dblock
Copy link
Member

dblock commented Jan 25, 2016

So if in after you return anything, that's what's returned from the API. I think that's OK, otherwise it feels too magical. Curious what others think.

I see what you mean though, but I think adding more "magic" to it isn't better.

@namusyaka
Copy link
Contributor Author

@dblock Yeah I see. I came to conclusion that your opinion is reasonable, but I'd like to hear other opinions.

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

Successfully merging this pull request may close these issues.

2 participants