Permalink
Browse files

Working version of a hefty `autoptimizeStyles::minify()` rewrite + wh…

…itespace/cs/comments:

* A lot more robust CDN replacement + now matches local font files too (not only background images)
* DRY-ed up some code, removed multiple code paths with duplicated code
* improved performance by moving certain parts out of the loop and/or making sure certain operations are not done multiple times when filtering/excluding stuff
* added some comments/docs, more to come

Needs testing, especially of the `datauri` inilining option.
CDN replacements have been tested extensively (albeit on a more-or-less stock WP setup -- some exotic subfolder and/or multisite installs might barf, although I'm not sure they would've worked without problems in the old version either)
  • Loading branch information...
zytzagoo committed Mar 9, 2015
1 parent 801a2cb commit 4b4f29a7606009804e3cf7bfb38d4280bfecb088
Showing with 638 additions and 542 deletions.
  1. +638 −542 classes/autoptimizeStyles.php
Oops, something went wrong.

5 comments on commit 4b4f29a

@futtta

This comment has been minimized.

Owner

futtta replied Nov 26, 2015

this is a very interesting change @zytzagoo , were you able to test this thoroughly already?

between march & now some changes were done to CDN (e.g. 2c1e94a) & local fonts (albeit behind a filter; 7218962), to what extent have you reviewed/ incorporated those changes?

and more generally speaking; feel free to issue pull requests, even if only for the whitespace/ comments/ ... changes. would be a pity if your work wouldn't benefit others as well, no?

and even more generally speaking; I can always use a real developer (which I am not) to keep things sane! ;-)

frank

@zytzagoo

This comment has been minimized.

Collaborator

zytzagoo replied Nov 26, 2015

Hey, I've been trying to keep up with upstream changes in my branch here: https://github.com/zytzagoo/autoptimize/tree/cdn-rewrite
https://github.com/zytzagoo/autoptimize/commits/cdn-rewrite

As for testing, my cdn-rewrite branch has been running in production on several sites, but that is no guarantee of any kind (I might've broken something but my sites aren't using it etc.)

I've wanted to do a PR (after polishing/maybe-rebasing, when time permits) ever since I forked it, but:

  • you keep adding new stuff :)
  • there is no test suite :/
  • work + parenting = very-little-or-no-free-time-at-all
@futtta

This comment has been minimized.

Owner

futtta replied Nov 26, 2015

re new stuff: yeah, preparing for AO 2.0, doing at least one commit a day now. but I think I'm almost there (except for bug fixes which are sure to come as I get people to test)

re test suite: indeed. have no knowledge/ experience there, something you might be interested in writing maybe?

re work+parenting: I feel your pain. only 1 daughter, 9yo. and a full-time job which is not related to wordpress. but it is my main hobby, so :-)

hope you'll find time to join in on the fun at some point in time, by the looks of it (your code) I think you could be a real asset to improve AO!

frank

@zytzagoo

This comment has been minimized.

Collaborator

zytzagoo replied Nov 26, 2015

Thanks for the kind words and I hope I'll do more with AO in the near future too!

tests: I might have some code laying around which I used to quickly test stuff while rewriting cdn handling -- the main problem here is really getting the AO code base to a point that would make certain things easily unit-testable (ie., markup replacements, url rewriting, etc.), and that's no small task -- I ain't promising anything :)

work+parenting: pretty much in the same boat, except our son will be 5 months old in a week or so :)

@futtta

This comment has been minimized.

Owner

futtta replied Nov 26, 2015

5 months, I vaguely remember that, a rollercoaster, do enjoy!

lets keep in touch!
frank

Please sign in to comment.