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

Add ImageMinify task #228

Merged
merged 1 commit into from
Oct 29, 2015
Merged

Conversation

gabor-udvari
Copy link
Contributor

Minifies images. When the required minifier is not installed on the
system the task will try to download it from the imagemin repository.

The task supports the following minifiers:

  • optipng
  • pngquant
  • advpng
  • pngout
  • zopflipng
  • pngcrush
  • gifsicle
  • jpegoptim
  • jpeg-recompress
  • jpegtran
  • svgo (only minification, no downloading)

Note before merging. The task works as intended, generated docs are included, the coding standard was checked. However the class is 603 lines of code and there are a few refactoring possibilities which you might want to consider:

  • I am not sure if the Robo\Task\Assets namespace is the right one, maybe a new Robo\Task\Images namespace would be more future proof.
  • The whole findFiles method is an exact copy from the FlattenDir task. There is already a discussion about adding glob support for all the tasks. When that happens the findFiles method can be replaced.
  • The task contains an installFromImagemin method which basically takes an URL, downloads the content and puts it into a file. Such a general download method could be created in the Robo\Task\Base namespace, maybe other tasks would benefit from it.

Minifies images. When the required minifier is not installed on the
system the task will try to download it from the imagemin repository.

The task supports the following minifiers:
- optipng
- pngquant
- advpng
- pngout
- zopflipng
- pngcrush
- gifsicle
- jpegoptim
- jpeg-recompress
- jpegtran
- svgo (only minification, no downloading)
@DavertMik
Copy link
Member

wow, that's great! Thank you, reviewing...

@DavertMik
Copy link
Member

Thanks, for now I will merge it as it is, but will keep on mind the refactoring ideas you proposed

DavertMik added a commit that referenced this pull request Oct 29, 2015
@DavertMik DavertMik merged commit 7553685 into consolidation:master Oct 29, 2015
@gabor-udvari gabor-udvari deleted the imageminify-task branch November 17, 2015 13:48
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

2 participants