remove explicit sass-rails dependency #250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jrochkind
force-pushed
the
remove_explicit_sass_rails_dependency
branch
from
7589425 to
f905b04
Compare
A Rails app is generated with a sass-rails dependency, so no changes to tests were needed -- the engine-cart-generated test app adds it's own sass-rails dependency. While current Rails releases generate with sass-rails dependency, sass-rails is sunsetted/deprecated, due to the underlying pure-ruby ruby-sass gem being deprecated, and instructs "consider switching to the sassc gem" https://github.com/sass/ruby-sass . Rails 6.0 will perhaps depend on the sassc-rails gem instead, or perhaps the sass-rails gem itself will be changed to depend on sassc instead of sass-ruby. I am not sure if it is yet determined/done. rails/rails#3289 But apps now may wish to switch to sassc-rails, especially if it's a new app. While sassc-rails _ought_ to be a drop-in replacement, bugs can happen, and the timeline of switching (or refraining from switching) should be up to a dependent app. However, if an app depends on both sass-rails and sassc-rails, problems can occur. sass/sassc-rails#6 So, removing the sass-rails as an explicit dependency from browse-everything will allow dependent apps to choose sass-rails or sassc-rails themselves, switching on their own timeline, without the browse-everything dependency interfering. This change is unlikely to pose any backwards compatibility problems, as dependent apps should have sass-rails in their Gemfile already, as it is generated by Rails. In the unlikely event some dependent app did not have sass-rails independently declared as a dependency, however, they would have to add it."
jrgriffiniii
approved these changes
Nov 19, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A Rails app is generated with a sass-rails dependency, so no changes to tests were needed -- the engine-cart-generated test app adds it's own sass-rails dependency.
While current Rails releases generate with sass-rails dependency, sass-rails is sunsetted/deprecated, due to the underlying pure-ruby ruby-sass gem being deprecated, and instructs "consider switching to the sassc gem" https://github.com/sass/ruby-sass .
Rails 6.0 will perhaps depend on the sassc-rails gem instead, or perhaps the sass-rails gem itself will be changed to depend on sassc instead of sass-ruby. I am not sure if it is yet determined/done. rails/rails#3289
But apps now may wish to switch to sassc-rails, especially if it's a new app. While sassc-rails ought to be a drop-in replacement, bugs can happen, and the timeline of switching (or refraining from switching) should be up to a dependent app. However, if an app depends on both sass-rails and sassc-rails, problems can occur. sass/sassc-rails#6
So, removing the sass-rails as an explicit dependency from browse-everything will allow dependent apps to choose sass-rails or sassc-rails themselves, switching on their own timeline, without the browse-everything dependency interfering. This change is unlikely to pose any backwards compatibility problems, as dependent apps should have sass-rails in their Gemfile already, as it is generated by Rails. In the unlikely event some dependent app did not have sass-rails independently declared as a dependency, however, they would have to add it.
Ref #246