Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

update go and swap to graphicsmagic #108

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

erulabs
Copy link

@erulabs erulabs commented Oct 26, 2016

  • Updates go to 1.8
  • Swaps to graphicsmagick gm convert instead of imagemagick

@pastudan
Copy link
Contributor

👍

@briankassouf
Copy link
Contributor

Hey @erulabs! If you switch to gm you no longer need the imagemagick policy:
https://github.com/Imgur/mandible/blob/master/docker/imagemagick_policy.xml

@erulabs
Copy link
Author

erulabs commented Oct 26, 2016

Hey @briankassouf - thanks haha :) Hope all is well!

@@ -6,7 +6,7 @@ import (
"github.com/Imgur/mandible/imageprocessor/thumbType"
)

const GM_COMMAND = "convert"
const GM_COMMAND = "gm convert"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the exec command will like this space. You might need to change the runProcessorCommand so that it adds convert as the first item in the argument slice

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually the runProcessorCommand function is used more generically for other types of commands then gm. Perhaps a new function that wraps runProcessorCommand?

Copy link
Author

Choose a reason for hiding this comment

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

Hrm - yeah - I think i'll just add "convert" as an arg to each of those commands. That way runProcessorCommand stays generic and we can use other parts of gm easily if we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants