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 minimagick_cli option to choose graphicsmagick or imagemagick #67

Conversation

KBSchmidt
Copy link

Added minimagick_cli param. By default, the image lib remains graphicsmagick, as I didn't want to brake any releases.

I did not compare the image outputs of the two cli's directly.

I'm still not sure why graphicsmagick is the deafault cli for the plugin, as minimagic's default cli should be imagemagick. If I uninstall graphicsmagick, it still tries to call gm identify ....

Still, using imagemagick should results in a faster build time on Bitrise, or any other CI/CD solution, which has imagemagick preinstalled, because you'd no longer have to install graphicsmagic.

Let me now what needs changing, I'm quite new to Ruby.

  • Updated .rubocop.yml as suggested by running rake
  • Updated Fastfile as suggested by running rake
  • Added minimagick_cli optional param, which can be 'graphicsmagick', 'imagemagick', 'auto'. Default to 'graphicsmagick'

Kornél Schmidt added 4 commits March 5, 2021 22:08
** updated rubocop.yml
** added  missing frozen string literal comment to fastfile
** Style/SymbolArray: Use %i or %I for an array of symbols
** removed deprecated Style/BracesAroundHashParameters
@joshdholtz joshdholtz changed the title Add imagemagick as an option Add minimagick_cli option to choose graphicsmagick or imagemagick Mar 25, 2021
Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for making this PR! This is a great addition and was a fun one to test (since I didn’t know much about this) 😊

I added a commit to change things up a little bit. Hope that’s okay 😇

  • minimagick_cli is optional that allows and verify graphicsmagick and imagemagick
  • The helper looks for either of those otherwise defaults to auto

Defaulting the option to graphicsmagick actually broke things for me because I didn’t have it installed. I only had imagemagick installed. So I wanted to let minimagick still pick auto unless explicitly set by the user.

Thanks again for this! ❤️ I will get this released today 💪

@joshdholtz
Copy link
Member

@KBSchmidt Also... fun thing I just discovered 👇

[!] `gm mogrify -alpha transparent -background none -fill white -draw roundrectangle 1,1,190,190,95,95 /var/folders/_g/0p2cz2md329dm9tz3p1wt2y40000gn/T/mini_magick20210325-18403-1wpzxmr.png` failed with error: (MiniMagick::Error)
gm mogrify: Unrecognized option (-alpha).

I got an error while making an Android icon while specifying graphicsmagick 😛 It looks like alpha is not supported. So when little MiniMagick auto pick it will pick based on what is best supported for the operation that it’s doing 🤷‍♂️

@joshdholtz joshdholtz merged commit e8c65cc into fastlane-community:master Mar 25, 2021
@KBSchmidt
Copy link
Author

My pleasure 😄 I hope it won't break anything. Good to know, how the verify block works 👍 Intereseting that graphicsmagick does not support alpha. Here's the issue: #37. So I guess this solves that too?

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.

2 participants