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 CDN usage #570

Merged
merged 1 commit into from
Nov 22, 2013
Merged

Fix CDN usage #570

merged 1 commit into from
Nov 22, 2013

Conversation

pborreli
Copy link
Contributor

@pborreli pborreli commented Nov 9, 2013

IMHO CDN assets shouldn't be download by assetic and combined.
Right now even if they are combined, browser still downloads linked images anyway ..

@cordoval
Copy link
Contributor

cordoval commented Nov 9, 2013

what about downloading them and making them part of the repo with bower/grunt or even component and combine them truly?

@pborreli
Copy link
Contributor Author

pborreli commented Nov 9, 2013

@cordoval that could possibly encourage people to make changes directly in files (example edit bootstrap css) instead of using their own css

@cordoval
Copy link
Contributor

cordoval commented Nov 9, 2013

?? no @pborreli those files are symlinked or moved with bower/component, they are not versioned together, and in any case the approach should make clear they shouldn't be changing cdn files. I mean it is not the greatest practice anyway and for that they have frontend.css

@pborreli
Copy link
Contributor Author

pborreli commented Nov 9, 2013

well as you want, my preference is CDN way but it's only my own point of view

@cordoval
Copy link
Contributor

cordoval commented Nov 9, 2013

lol, not against your view, just my view too, if say i am in an airplane or somewhere remote without wifi then i don't need it to develop. But is not as I wish, but as they wish 👶 Let's wait and see.

@pborreli
Copy link
Contributor Author

pborreli commented Nov 9, 2013

if you are in an airplane without wifi, you can't dump assets as Assetic will try to download files from CDN to combine them.

@cordoval
Copy link
Contributor

cordoval commented Nov 9, 2013

correct, but if they are already downloaded with composer+component then they will just be added and combined right? of course you ran composer before while on earth.

@pborreli
Copy link
Contributor Author

pborreli commented Nov 9, 2013

right now the combine is only made during sylius:install not by run-scripts during composer update/install but yes, once it's done, you don't need internet anymore if you don't make any changes in css/js

@pjedrzejewski
Copy link
Member

I'd love to do it right, with Bower... but this introduces requirement for node.js on the server, am I right?

@cordoval
Copy link
Contributor

not with component from @RobLoach, is one of drupal's smartest guys like @Crell 👶 so we should ponder it.

stloyd added a commit that referenced this pull request Nov 22, 2013
@stloyd stloyd merged commit c3a7393 into Sylius:master Nov 22, 2013
@pborreli pborreli deleted the fix-cdn-usage branch November 22, 2013 15:33
@pborreli
Copy link
Contributor Author

🐼

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.

4 participants