also call after_any_fetch after user is set -> after_change_user #55

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@grosser
Contributor

grosser commented Jun 7, 2012

I missed the point that users can also be just set in my first pull,
so I renamed the hook to reflect that its called after user was changed,
and added the callback to set_user and user + made sure it is not called twice

(our app has to change locale whenever user is changed)

@hassox

This comment has been minimized.

Show comment Hide comment
@hassox

hassox Jun 8, 2012

@josevalim any issues with this one mate?

hassox commented on 411e343 Jun 8, 2012

@josevalim any issues with this one mate?

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 8, 2012

Collaborator

I am fine with this change but @grosser wouldn't be better to simply have a callback for whenever fetch fails? We already have one for whenever fetch succeeds, so instead of having another callback overlapping, couldn't we simply have the counterpart?

Collaborator

josevalim commented Jun 8, 2012

I am fine with this change but @grosser wouldn't be better to simply have a callback for whenever fetch fails? We already have one for whenever fetch succeeds, so instead of having another callback overlapping, couldn't we simply have the counterpart?

@grosser

This comment has been minimized.

Show comment Hide comment
@grosser

grosser Jun 8, 2012

Contributor

I think that's a good idea, the combination will capture everything,
how about after_fetch_fail ?

Contributor

grosser commented Jun 8, 2012

I think that's a good idea, the combination will capture everything,
how about after_fetch_fail ?

@josevalim

This comment has been minimized.

Show comment Hide comment
@josevalim

josevalim Jun 8, 2012

Collaborator

Sounds great. Can we close this then and expect a new PR from you? :) Thanks.

Collaborator

josevalim commented Jun 8, 2012

Sounds great. Can we close this then and expect a new PR from you? :) Thanks.

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