Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add Rack::Response for Rack::Auth::Basic #433

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants

I have an application which needs to know the current requested path to know which user is allowed or not.
I think knowing the current request is a generic need.

Regards.

Member

rkh commented Sep 22, 2012

Why is it a splat? Could you add tests?

Sorry, I'm not english, I don't understang your first sentence.
I add tests, sorry for the delay, I first had to discover RSpec.

Contributor

gioele commented Sep 24, 2012

@blackheaven a splat is an array that is expanded so that each item in it ends up as a parameter in a function call. In your case the splat you added is *auth.request. In the code you wrote you are not passing the request object but the content of the request object.

Oh, ok, thanks, we don't have specific word in french, the answer is : it is useless.

Contributor

gioele commented Sep 25, 2012

@blackheaven if you removed the splat and the tests kept on working it is very likely that the tests where not testing the feature you implemented in the correct way. It looks like you never test whether the received response object is really an acceptable response object (this kind of test would have failed in the previous version, should pass after your latest commit).

The tests didn't verify the new parameter, I add it.
Thanks.

Owner

raggi commented Oct 31, 2012

Patch does not support 1.8 which cannot have left hand splats.

I substitute the splat by a hard coded solution.

Owner

raggi commented Nov 2, 2012

I'm discussing this with @blackheaven on IRC. I'm concerned about altering the arity, as it can cause ArgumentError for users that have passed arity specific authenticators.

I recommended this approach: https://gist.github.com/3af15d688e3ace9d31a5

@raggi raggi closed this Nov 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment