search

foundation / foundation-rails

Foundation for Rails
foundation.zurb.com
MIT License
1k stars 376 forks source link

Changes the require to the correct sprocket gem #274

Closed brianmcoates closed 5 years ago

brianmcoates commented 5 years ago

This will fix the issue that breaks everything when trying to start your rails server. I have created an issue as well that this would close #273. I saw another fix was to add sprockets es6 to the gem file as well if that is the desired fix I can submit a pull request to update the documentation.

khiet commented 5 years ago

Hi @ncoden,

I see three options.

⚠️ I don't know what sprockets-es6 is for.

  1. Update README to let users know gem 'sprockets-es6' has to be added to their Gemfile.

  2. Add back sprockets-es6 in the gemspec file so that it gets installed as a dependency.

  3. if sprockets-es6 is an unnecessary dependency for this gem, these gemfiles need to be cleaned up and @brianmcoates's fix is nice since Rails already comes with sprockets-rails.

My preference is 2.

brianmcoates commented 5 years ago

@ncoden after talking to my team here we found this https://github.com/zurb/foundation-rails/commit/a6c927db5f604d8d3e2e741389455b69d4c7aff2#diff-44263267796d33ed2765ce27a5ba5f3c What is happening when people bundle install bundle doesn't know what the dependencies are so we need to add them back in. I have updated the pull request to do that.

rafibomb commented 5 years ago

I'll try to pull our Rails engineer in to check it out.

ncoden commented 5 years ago

@khiet @brianmcoates Thanks a lot for the explanations!

I made a mistake, I am sorry. Now I understand better how gemspec files are used. The pull request looks good to me now, but I'm waiting for the opinoin of @rafibomb's Rails engineers to merge it.

brianmcoates commented 5 years ago

@ncoden No worries man it happens to everyone. :) Looking forward to hearing back from your guys rails engineer.

brianmcoates commented 5 years ago

@ncoden have you guys had a chance to talk to your rails eng?

ncoden commented 5 years ago

@rafibomb

javierjulio commented 5 years ago

@ncoden the right thing to do here would be to just include the following 3 gemspec dependencies:

spec.add_dependency "sass", [">= 3.3.0", "< 3.5"]
spec.add_dependency "railties", [">= 3.1.0"]
spec.add_dependency "sprockets-es6", [">= 0.9.0"]

again and release that now rather than wait. The current release is unusable because of that regression. You can at least temporarily fix that and release again to address changing dependencies.

patricklindsay commented 5 years ago

Where is this Rails engineer at @rafibomb @ncoden ? :)

ncoden commented 5 years ago

I removed the upper limit for sass (foundation-sites support all latest Sass versions) and regenerated the lockfiles.