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

Fix issue #4 #5

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

Fix issue #4 #5

wants to merge 3 commits into from

Conversation

haakym
Copy link

@haakym haakym commented Nov 15, 2016

For issues #3 and #4

Would appreciate feedback for src/Converter/GhostscriptConverterCommand.php, not an ideal solution and more of a proof of concept. Perhaps the whole guess the GhostScript command by OS should be extracted to an external class? Scratching my head on how to write tests for this also!

@xthiago xthiago changed the title Issues #3 and #4 Fix issue #4 Feb 13, 2018
@xthiago
Copy link
Owner

xthiago commented Feb 13, 2018

@haakym
I renamed the pull-request name because the issue #3 has been solved by PR #16 (and released under v1.0.5).

I merged master into your branch and I resolved the conflicts. I think we can go ahead with this PR.

I have only one question: on Windows the Ghostscript come with system or need to be installed? Could be usefull to add install instructions on README.md.

@edegaudenzi Do you have suggestions to also support macOS here?

@edegaudenzi
Copy link
Contributor

-MacOS
@xthiago I am currently using gs on a mac, the easiest way is with brew (https://brew.sh/), so:

brew install ghostscript

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.

3 participants