8000 Add switch to cli to allow choice between Chrome/Firefox by jesse-osiecki · Pull Request #38 · breenmachine/httpscreenshot · 8000 GitHub
[go: up one dir, main page]

Skip to content

Add switch to cli to allow choice between Chrome/Firefox#38

Merged
breenmachine merged 6 commits intobreenmachine:masterfrom
jesse-osiecki:browser-flag
Jan 27, 2022
Merged

Add switch to cli to allow choice between Chrome/Firefox#38
breenmachine merged 6 commits intobreenmachine:masterfrom
jesse-osiecki:browser-flag

Conversation

@jesse-osiecki
Copy link
Contributor
@jesse-osiecki jesse-osiecki commented Jan 26, 2022
  • Add switch to cli to allow choice between Chrome/Firefox, keeping the -p headless flag as an option for both
  • Default is Firefox as the previous changes defaulting to Chrome breaks previous functionality, Docker image doesn't have Chrome
  • the install script is used by the Docker image, which doesn't have sudo. I propose reverting the addition of sudo within the script, and rather added verbiage to the README and script to run it with sudo

@jesse-osiecki
Copy link
Contributor Author

@breenmachine @jstnkndy I propose adding a switch to the tool to allow a choice of browsers. I use Firefox headless so I was surprised when I pulled master and got Message: unknown error: cannot find Chrome binary

Currently I don't bake Chrome webdriver into the docker image, and while I am not opposed, I made this commit to keep the original functionality while extending the improvements that Justin made.

jesse-osiecki and others added 5 commits January 26, 2022 18:07
Changing dockerfile comment pointing to https://hub.docker.com/r/andmyhacks/httpscreenshot as it's 5 years out of date
  -p headless flag as an option for both

  Default is Firefox as the previous changes defaulting to Chrome breaks
  previous functionality
@breenmachine
Copy link
Owner

Hey Jesse, thanks so much for the PR. I agree it's nice to have both options. Merging it now.

@breenmachine breenmachine merged commit 0ef8f8f into breenmachine:master Jan 27, 2022
@jesse-osiecki
Copy link
Contributor Author

Thanks @breenmachine !

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

0