Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

process mini magic commands with combine_options #1754

Merged

Conversation

bernabas
Copy link
Contributor

No description provided.

@bernabas
Copy link
Contributor Author

hey @bensie @eavgerinos, can you take a look at this when you have time? i made some changes for mini magick processings to take advantage of combine_options :) thanks

@bernabas
Copy link
Contributor Author

more comments .... so basically, previously we would have to do

resize_to_limit(200, 200) do |img|
  img.quality 90
end

which would generate two commands

[0.01s] mogrify -resize 130x130> <blah blah file name>
[0.01s] mogrify -quality 90  <blah blah file name>

but with this change, we would do

resize_to_limit(200, 200, combine_options: {quality: 90})

and it generates one command

[0.01s] mogrify -resize 130x130> -quality 90  <blah blah file name>

@abookyun
Copy link

Hence combine_options is more efficient way to execute commands, I'll put 👍 on this.

@thomasfedb
Copy link
Contributor

Can we get this merged @bensie?

bensie added a commit that referenced this pull request Nov 27, 2015
process mini magic commands with combine_options
@bensie bensie merged commit 8232a38 into carrierwaveuploader:master Nov 27, 2015
@bensie
Copy link
Member

bensie commented Nov 27, 2015

Thanks @bernabas!

@bernabas
Copy link
Contributor Author

@bensie no problem, thanks for the merge

mehlah pushed a commit to mehlah/carrierwave that referenced this pull request Dec 29, 2015
This method isn't intended to be part of the public API.
It's used internally by the processor to accept `combine_options`
introduced at carrierwaveuploader#1754.
mehlah pushed a commit to mehlah/carrierwave that referenced this pull request Dec 29, 2015
Since carrierwaveuploader#1754 merge, JRuby builds fails due to the new hash syntax.
See https://travis-ci.org/carrierwaveuploader/carrierwave/jobs/93603851
This is due to JRuby mode. `jruby` resolves to 1.7.10 in 1.9 mode and
`jruby-head` resolves to the JRuby master branch in 2.1 mode.
As Travis doesn't support d20 and d21 for JRuby (see travis-ci/travis-ci#2432 (comment)),
using `JRUBY_OPTS` in the build matrix fixes the modes to run.
This matches MRI Ruby minimal requirements for CarrierWave 1.0.0.
mehlah pushed a commit to mehlah/carrierwave that referenced this pull request Dec 29, 2015
Since carrierwaveuploader#1754 merge, JRuby builds fails due to the new hash syntax.
See https://travis-ci.org/carrierwaveuploader/carrierwave/jobs/93603851
This is due to JRuby mode. `jruby` resolves to 1.7.10 in 1.9 mode and
`jruby-head` resolves to the JRuby master branch in 2.1 mode.
As Travis doesn't support d20 and d21 for JRuby (see travis-ci/travis-ci#2432 (comment)),
using `JRUBY_OPTS` sets the mode to run for JRuby 1.7.
This matches MRI Ruby minimal requirements for CarrierWave 1.0.0.
mynock pushed a commit to mynock/carrierwave that referenced this pull request Aug 9, 2016
This method isn't intended to be part of the public API.
It's used internally by the processor to accept `combine_options`
introduced at carrierwaveuploader#1754.
mynock pushed a commit to mynock/carrierwave that referenced this pull request Aug 9, 2016
Since carrierwaveuploader#1754 merge, JRuby builds fails due to the new hash syntax.
See https://travis-ci.org/carrierwaveuploader/carrierwave/jobs/93603851
This is due to JRuby mode. `jruby` resolves to 1.7.10 in 1.9 mode and
`jruby-head` resolves to the JRuby master branch in 2.1 mode.
As Travis doesn't support d20 and d21 for JRuby (see travis-ci/travis-ci#2432 (comment)),
using `JRUBY_OPTS` sets the mode to run for JRuby 1.7.
This matches MRI Ruby minimal requirements for CarrierWave 1.0.0.
@krismartin
Copy link
Contributor

@bernabas How can I use the combine_options to append a "+" operator (e.g. +antialias)?

https://github.com/minimagick/minimagick/blob/master/lib/mini_magick/tool.rb#L163

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants