search

rails / sass-rails

Ruby on Rails stylesheet engine for Sass
MIT License
860 stars 333 forks source link

Prevent beta versions of gems as a dependency #387

Closed doits closed 8 years ago

doits commented 8 years ago

Before, the version dependency for example on sprockets was like this:

s.add_dependency 'sprockets',       '>= 2.8', '< 4.0'

But this allowed to install beta versions of sprockets like current 4.0.0.beta3 (because 4.0.0.beta3 < 4.0). Which is not compatible with sass-rails.

By using < 4.x, it successfully prevents to install any version 4 of sprockets, including beta versions.

Other gem dependencies changed accordingly.

rails-bot commented 8 years ago

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

rails-bot commented 8 years ago

warning Warning warning

matthewd commented 8 years ago

Why?

A beta should only get installed if you specifically asked for it (or depended on something that did) :confused:

doits commented 8 years ago

True, if I always only use released versions of everything, it will not bite me.

But simple example when I have in my Gemfile - because maybe I want to test latest sprockets:

gem 'sprockets', '~> 4.x'
gem 'sass-rails'

... then sass-rails 5.0.6 gets installed which is not compatible with sprockets 4.x. But bundler thinks it is compatible, because that is what < 4.0 means afaik.

It should tell me instead that it cannot satisfy dependencies for sprockets ~> 4.x because there is no sass-rails version compatible with it (yet). Which is the case, or not?

And besides the incompatibility with beta versions, semantic meaning of < 4.0 is also "I am compatible with 4.0.beta1". But it is not afaik? This might be a little bit picky, I know :-)

IMO you only could say "We do not care about beta versions, who uses them must see for himself" but I think such a small change would not hurt anyway.

So this is just a small PR to help out all the beta users :-) (including me of course)

matthewd commented 8 years ago

gem 'sprockets', '~> 4.x'

Ah, so you've explicitly opted in to beta versions.

Firstly, we can't really fix this now: there is a version out there that believes itself compatible with 4.0.beta1, and that's not going away. If we released the above change as 5.0.7, you'd just still get 5.0.6 installed.

As a more general observation, though, I prefer to leave these dependencies in this "if you force it, we'll allow it" state, because it makes it much more approachable for a beta user to try it out, and then report whether things need to be changed to actually be compatible with the upcoming version when it's released (or alternatively, whether we just need to open up the dependency with no other changes).

So, it's not quite "we don't care"... there are just competing goals, and incomplete knowledge at the time the decision is made. If we could retroactively adjust dependency ranges to reflect new information after other versions were released, it might be different, but as it is, this seems a sensible balance. (So too if bundler allowed a Gemfile to force installation of something that might be incompatible, but that's not going to happen.)