Skip to content
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

Include main.scss through the asset pipeline path resolver #4344

Merged
merged 1 commit into from Jul 24, 2018

Conversation

skateman
Copy link
Member

When sprockets loads a file using the require directive, it usually goes through the config.assets.paths and searches for the given file. However, if you prepend the filename with a ./ as we did in f18e5d6 it will search only in the directory of the file with the directive.

This is all good until you don't want to override a file from a location with higher priority...

@miq-bot add_label bug
@miq-bot add_reviewer @epwinchell
@miq-bot add_reviewer @himdel

@miq-bot miq-bot added the bug label Jul 24, 2018
@miq-bot
Copy link
Member

miq-bot commented Jul 24, 2018

Checked commit skateman@c71b241 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@epwinchell
Copy link
Contributor

cc @Fryguy

@himdel
Copy link
Contributor

himdel commented Jul 24, 2018

LGTM, merging when green :)

(Though .. if we ever add a npm package called main, this will break. (Until we can drop node_modules from asset_paths again.))

@himdel himdel self-assigned this Jul 24, 2018
@himdel himdel added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 24, 2018
@skateman
Copy link
Member Author

@himdel there's no such package in the NPM registry and let's hope it will not happen 😉

@himdel
Copy link
Contributor

himdel commented Jul 24, 2018

Creating one in a minute.. :trollface:

@skateman
Copy link
Member Author

@himdel make sure it won't do anything useful, so we will never use it 😉

@himdel himdel merged commit 2f350d0 into ManageIQ:master Jul 24, 2018
@skateman skateman deleted the main-nonrelative branch July 24, 2018 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants