New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Rails example documentation. #22

Merged
merged 2 commits into from Oct 10, 2016

Conversation

Projects
None yet
2 participants
@f96q
Copy link
Contributor

f96q commented Oct 7, 2016

Rails production environment default set Rails.application.config.assets.compile to false.
The Rails.application.assets return nil from sprockets-rails version3.
This case not use Rails.application.assets.find_asset.

rails/sprockets-rails#237
rails/sprockets-rails#294

@jgarber623
Copy link
Owner

jgarber623 left a comment

Thanks for the documentation update, @f96q! That's a solid update and will help those of us migrating apps from Rails 4 to 5.

I posted a couple comments based on what I found reading through the Sprockets changes you linked to.

If you can make those updates, I'll go ahead and merge your PR. Thanks again!

README.md Outdated
<%= raw Rails.application.assets.find_asset('icons.svg') %>
<% else %>
<%= raw Rails.application.assets_manifest.find_sources('icons.svg').first %>
<% end %>

This comment has been minimized.

@jgarber623

jgarber623 Oct 9, 2016

Owner

According to this comment (and confirmed by my own testing), find_sources will work in both development and production. So in a Rails 5 application, this block could be shortened to:

<%= raw Rails.application.assets_manifest.find_sources('icons.svg').first %>
README.md Outdated
@@ -107,13 +107,17 @@ For example, a file named `menu.svg` in `~/Sites/sixtwothree.org/images/icons` w

To use an svgeez-generated SVG sprite file, first include the file's contents at the bottom of your HTML page.

In a Rails 4 application:
In a Rails 5 application:

This comment has been minimized.

@jgarber623

jgarber623 Oct 9, 2016

Owner

Since there are still plenty of Rails 4 applications out in the wild, it would be helpful to have both Rails 4 and Rails 5 examples in the README. Could you retain the Rails 4 example and add your Rails 5 example?

@f96q f96q force-pushed the f96q:rails-readme branch from 69129df to 146f788 Oct 10, 2016

@jgarber623 jgarber623 merged commit 146f788 into jgarber623:master Oct 10, 2016

2 checks passed

codeclimate Code Climate didn't find any new or fixed issues.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jgarber623

This comment has been minimized.

Copy link
Owner

jgarber623 commented Oct 10, 2016

Thanks, @f96q! Merged your PR and added you to the list of contributors in the README. 👍

@f96q

This comment has been minimized.

Copy link
Contributor Author

f96q commented Oct 10, 2016

thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment