dlindahl / omniauth-cas

A CAS OmniAuth Strategy
MIT License
88 stars 79 forks source link

Relax version requirements; support Rails 5 #43

Closed lucaspiller closed 8 years ago

lucaspiller commented 8 years ago

Relaxes the dependency on Omniauth, to allow 1.3 to be installed which is required for Rails 5.

dlindahl commented 8 years ago

If you can fix the build error, I will blindly merge this in. Something to do with minimum required Ruby version in the .travis.yml file.

vasilakisfil commented 8 years ago

@lucaspiller bump

lucaspiller commented 8 years ago

Ok fixed, as there is no Gemfile.lock it was trying to install the newest version of Rack which requires Ruby 2.2.2+. Apparently Bundler isn't smart enough to figure that out? I've just changed the Gemfile to take into account that logic. For a gem I'm not sure whether a Gemfile.lock is a good idea - on one hand it prevents these issues, but on the other it also means that it won't test compatibility against the latest and greatest dependencies.

lucaspiller commented 8 years ago

Also @dlindahl I saw your comment on the other PR about not having access to a CAS server to test and maintain this project. At IFAD we are big users of CAS and proponents of open source, so we can probably take over. Ping @vjt!

sahglie commented 8 years ago

+1

mpeteuil commented 8 years ago

@lucaspiller The Gemfile.lock file is generally not checked in when the project in question is a gem. See Yehuda's post Clarifying the Roles of the .gemspec and Gemfile for more details.

If there's anything I can do to help get this in let me know.

Cheers

dlindahl commented 8 years ago

Few more changes:

Do that and I'll merge this in and make you a contributor.

lucaspiller commented 8 years ago

I'd prefer not to drop support for Ruby < 2.2.2 as we still have a large number of applications running on Ruby 2.1, it's still a supported Ruby version until probably next year at least.

The original issue of the specs failing is actually a missing feature of Bundler, it doesn't check or respect the required_ruby_version option in the Gemfile:

I find it hard to believe this is the first Gem that has come across this issue - does anyone know how other projects have handled this?

dlindahl commented 8 years ago

Upgrade your apps then. 😜

LGTM!

vjt commented 8 years ago

Hey @dlindahl,

if it were that easy to just upgrade :-). Thanks for keeping Ruby 2.1 supported in the latest omniauth-cas, at least as long as it is supported upstream.

@lucaspiller, thanks for the ping, I will provide the project a test infrastructure for CAS authenticating against an OpenLDAP. Summer is coming, and with that lots of heat and free time to play with UNIX toys.

Cheers! 🍻

dlindahl commented 8 years ago

@lucaspiller ping me with your RubyGems email address so I can add you as an owner of the gem

mpeteuil commented 8 years ago

@lucaspiller @dlindahl Thoughts on a bumping the version with these changes? If so, following semver would that be a major version change with the support for some older rubies being dropped?

dlindahl commented 8 years ago

You are contributors, go for it! I just need some emails to add to RubyGems so you can push the gem out. I'm out of service until Monday though

On August 24, 2016 at 7:03:51 AM, Michael Peteuil (notifications@github.com) wrote:

@lucaspiller https://github.com/lucaspiller @dlindahl https://github.com/dlindahl Thoughts on a bumping the version with these changes? If so, following semver would that be a major version change with the support for some older rubies being dropped?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dlindahl/omniauth-cas/pull/43#issuecomment-242075886, or mute the thread https://github.com/notifications/unsubscribe-auth/AADaicWDvI_qP7SoOh2piH612blHJb5jks5qjE9HgaJpZM4JID4p .

lucaspiller commented 8 years ago

@dlindahl Oops, sorry missed this! Here you go:

luca@stackednotion.com vjt@openssl.it

I'll make a release once you've added us, as the current version doesn't include this change so won't work with Rails 5. Thanks!

lucaspiller commented 8 years ago

@mpeteuil This change only lifts a restriction in the gemspec, there are no actual code changes, so old versions of Ruby should still work.

dlindahl commented 8 years ago

@mpeteuil @lucaspiller @vjt: I've added you all as owners of this gem: https://rubygems.org/gems/omniauth-cas

lucaspiller commented 7 years ago

Thanks @dlindahl. Can you give us commit access to this repo, or we can fork it to our organisation if you want to step away.

dlindahl commented 7 years ago

To prevent URL breakage, I'd recommend keeping it here. @lucaspiller, you should have full repo access, but GH says you have not accepted the Collaborator's invite. Check your email. I can resend if you can't find it. Let me know!

lucaspiller commented 7 years ago

Ah I can't find it, must have deleted it accidentally :no_mouth:

dlindahl commented 7 years ago

Re-sent!

lucaspiller commented 7 years ago

Ok, I'm in. v1.1.1 is away! Pushed and tagged here, and new gem pushed to RubyGems. 🎆 🎉 💃