-
-
Notifications
You must be signed in to change notification settings - Fork 150
Conversation
WOW! Nice list :) Especially the last one heh heh |
Hah yeah :D Most of the list I think is already done, I'm just double checking it an making some final decisions. I'd love reviewers when it's wrapped up as I'm still a bit of a gulp newb |
Great. I think I can help you as much as I know. |
Great stuff! But I'd be careful about LibSASS these days. I recently switched to Ruby SASS, since LibSASS isn't able to compile |
@amannn thanks for the heads up on LibSass. I may remove the option for libsass until that bug is fixed. |
Also CSSO isn't able to compile |
OK, I think I've addressed all the major issues. @addyosmani @sindresorhus if either of you have time to review this before I merge I would be most appreciative. The one thing I was not able to figure out was how to merge the |
|
||
// Run PageSpeed Insights | ||
// Update `url` below to the public URL for your site | ||
gulp.task('pagespeed', pagespeed.bind(null, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update to the latest psi and see the updated usage: https://github.com/google/web-starter-kit/blob/ff33b9ab2af1005afbd704326fb484a6af502fee/gulpfile.js#L179
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should mostly be able to steal the above code snippet from our changes.
Just looking at the diff, lgtm. |
@JosefJezek We're using clean-css, not CSSO. |
@JosefJezek In Blink/Chrome, there are conversations on-going about whether or not we're going to keep @robdodson We'll review again post the PSI changes. |
Closed via 5011f42 |
This is a work in progress and not ready to be merged just yet :)
styles
andelements
task