Eliminate overlapping app/assets load path
#21794
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
jeremy
added a commit
to jeremy/sprockets-rails
that referenced
this pull request
Sep 28, 2015
`app/assets` was added to give `manifest.js` a place to live, but it suggests that `require 'stylesheets/foo.css'` might be the right way to refer to `require 'foo.css'`. We resolve the overlap and clarify that `manifest.js` is config by moving it from `app/assets/` to `app/assets/config/`. Put it up on a pedestal: rails/rails#21794 This reverts rails#232 and commit 39ecb27, reversing changes made to 3a44d47.
| @@ -1,4 +1,6 @@ | |||
| require 'concurrent' | |||
| # FIXME: clean up warning spam from concurrent lib | |||
There was a problem hiding this comment.
I think they are already fixed. Can we just point to master branch of it just to avoid having patches on Rails?
There was a problem hiding this comment.
The warnings are fixed, except for ruby-concurrency/concurrent-ruby#419
There was a problem hiding this comment.
I plan to publish a new build of concurrent-ruby today. That should fix the warnings. I'll create a Rails PR pointing to the new version as soon as I do.
jeremy
force-pushed
the
sprockets/move-asset-manifest-from-toplevel-to-config-subdir
branch
from
5fd64f8
to
10b43a2
Compare
|
|
| @@ -154,6 +154,11 @@ PATH | |||
| rake (>= 0.8.7) | |||
| thor (>= 0.18.1, < 2.0) | |||
|
|
|||
| PATH | |||
| remote: ~/source/concurrent-ruby | |||
There was a problem hiding this comment.
That doesn't sound quite right
jeremy
force-pushed
the
sprockets/move-asset-manifest-from-toplevel-to-config-subdir
branch
from
10b43a2
to
8736357
Compare
jeremy
added a commit
to jeremy/sprockets-rails
that referenced
this pull request
Sep 29, 2015
Bump version to 3.0.0.beta3 `app/assets` was added to give `manifest.js` a place to live, but it suggests that `require 'stylesheets/foo.css'` might be the right way to refer to `require 'foo.css'`. We resolve the overlap and clarify that `manifest.js` is config by moving it from `app/assets/` to `app/assets/config/`. Put it up on a pedestal: rails/rails#21794 This reverts rails#232 and commit 39ecb27, reversing changes made to 3a44d47.
* Move `app/assets/manifest.js` to `app/assets/config/manifest.js`. Avoid the suggestion that you can/should deep-link `stylesheets/foo`. * Pull in all toplevel stylesheets and JavaScripts, not just `application.js` and `.css`. Demonstrate how to use `link_directory` with a specified `.js`/`.css` type. * Fix RAILS_ENV handling in assets tests. * Shush warnings spam from third-party libs that distract from tests.
jeremy
force-pushed
the
sprockets/move-asset-manifest-from-toplevel-to-config-subdir
branch
from
8736357
to
20ec1e9
Compare
jeremy
added a commit
that referenced
this pull request
Sep 29, 2015
…rom-toplevel-to-config-subdir Eliminate overlapping `app/assets` load path
jeremy
deleted the
sprockets/move-asset-manifest-from-toplevel-to-config-subdir
branch
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.
app/assets/manifest.jstoapp/assets/config/manifest.js. Avoid the suggestion that you can/should deep-linkstylesheets/foo.application.jsand.css. Demonstrate how to uselink_directorywith a specified.js/.csstype.RAILS_ENVhandling in assets tests.app/assetstoplevel path in Drop toplevelapp/assetsload path sprockets-rails#277