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

Fix css loading and add logging #140

Closed
wants to merge 6 commits into from
Closed

Fix css loading and add logging #140

wants to merge 6 commits into from

Conversation

divoxx
Copy link

@divoxx divoxx commented May 28, 2015

This pull requests introduce a couple of small fixes around css loading to make sure that they are only loaded using one strategy. It also adds a debug logging to show which strategy was used.

See notes on the code.

reset_cache! if @cache.nil?
@cache
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache was spread through two different places, the old CacheLoader and this module, which kinda breaks encapsulation. Now cache is fully implemented here and a little bit more robust.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 50bc56f on doximity:fix_loading_and_add_logging into * on fphilipe:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 11c9769 on doximity:fix_loading_and_add_logging into * on fphilipe:master*.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 11c9769 on doximity:fix_loading_and_add_logging into * on fphilipe:master*.

@dmitry
Copy link

dmitry commented Aug 30, 2015

Looks good, please rebase. 👍

@aprescott
Copy link

Does this fix the timeout problems in #108? If so, I'd love to see this merged!

CSSLoaders::FileSystemLoader,
CSSLoaders::AssetPipelineLoader,
CSSLoaders::NetworkLoader
CSSLoaders::NetworkLoader,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comma left.

@fphilipe
Copy link
Owner

fphilipe commented Jan 6, 2016

I did some refactoring of how the cache works and thus some of this no longer applies. Closing, but feel free to reopen a new PR (ideally doing one change at a time).

@fphilipe fphilipe closed this Jan 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants