Skip to content

Conversation

yaiir-a
Copy link
Contributor

@yaiir-a yaiir-a commented May 16, 2024

Bumped version to avoid Node deprecation error.

It also worked fine for me without chromedriver-autoinstaller, so I removed it for simplicity's sake. Maybe theres something important there that Im missing.

@yaiir-a
Copy link
Contributor Author

yaiir-a commented May 16, 2024

Just found this recipe again, when trying to run selenium in headed mode on Github. Didn't realise that I've used it before AND I was the person who made the last PR 15 months ago 😆

@MarketingPip
Copy link
Member

Hey @yaiir-a!

Long time no see! hahah! Glad to see a PR from you again and glad to hear you made use of this. (Don't ask me how long I struggled way back to get this working - especially being more noobish then lol.)

That said - I want to address somethings so you know why I am going to deny this PR.

  1. The chromedriver-autoinstaller install option is useful - being it someone modifies the runner etc they are on (or maybe even actually uses this script as a template to build a app as executable etc). It will save them MUCH confusion.
  2. The above package also prevents need for setting the "Service" path.

Feel free to look more into the package if you are interested here.

Tho - if you want to earn some PR credit etc again. Feel free to send a PR with the bumped dependencies. And I will accept it hahah! Please make sure to add a emoji to the end of your commit message. If updating preferably :pencil:. Would be cool if we could figure out a way to auto-bump the dependencies in the YAML file tho. (If you want to take a shot at that - would be MORE than appreciated)

Regardless too if you make another PR etc (assuming you will haha!),
I do hope all is well! 👍

ps; pretty cool you actually stumbled back upon this!

@yaiir-a yaiir-a mentioned this pull request Jun 4, 2024
@yaiir-a
Copy link
Contributor Author

yaiir-a commented Jun 4, 2024

Apologies I missed your response!

As I was submitting the PR, I thought I should probably have made it as two separate PRs. But hey ho, you live and you learn!

Auto-bumping dependencies is probably beyond what I am able to do, but I did submit another PR with the version bump as a sign of my appreciation for your time in giving that response!

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