search

nbudin / devise_openid_authenticatable

OpenID authentication for Devise
MIT License
99 stars 32 forks source link

cleanup deprecation warnings #27

Closed ecoologic closed 8 years ago

ecoologic commented 8 years ago

I had some problems running the tests, which I solved looking at the other Gemfiles and adding the missing dependencies.

I'm not sure how I was supposed to use the Gemfile the way it was because Rails was not defined as a dependency anywhere.

Happy to change it if I get to grasp it.

At least I can run the specs now (:

nbudin commented 8 years ago

Hi, thanks for that! It's been so long since I set this up that I'm actually not sure what the deal with the Gemfile at the root is either, but it seems to be working this way so I don't see any problem with these changes.

ecoologic commented 8 years ago

@nbudin Thanks for that. I have another one, you can see the diff with your previous master here:

https://github.com/ecoologic/devise_openid_authenticatable/pull/1/files

I would love a hint to fix this build:

https://travis-ci.org/ecoologic/devise_openid_authenticatable/jobs/151983113

Basically, devise 3.4 (changelog: https://github.com/plataformatec/devise/blob/3-stable/CHANGELOG.md#340---2014-10-03) introduces some changes that break our build.

What I haven't understood so far is: Should I fix the spec or should I fix the gem itself?

I don't think remember_token is ever assigned.

Any suggestion is appreciated.

nbudin commented 8 years ago

I'd still like to test somehow that this doesn't break rememberable, so testing that the user is remembered seems like a good thing. But if this is no longer how Devise does it, then we'll have to fix the spec. (We can't fix the gem because the gem has really nothing to do with it.)

Looking at that changelog it's not obvious to me how Devise broke remember_token functionality, and it does seem to still be present in its codebase. Could you say more about the breakage?

ecoologic commented 8 years ago

I 100% agree with you, we need to keep the spec. I'm investigating the bug, but damn, it's fairly new territory. I'll get there and report back.

Thanks so much for your latest update, it's a very important confirmation.

I can't say much more so far, the only hint I have is that user.save is called twice, and the second time the token is saved on 3.3, but it is not saved on 3.5.

Hopefully, I'm still on top of it.

Thanks