Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Bump versions + remove unnecessary chromedriver-autoinstaller #5

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

Closed
yaiir-a wants to merge 3 commits into MarketingPipeline:main from yaiir-a:main

Conversation

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.

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 reacted with thumbs up emoji MarketingPip reacted with laugh emoji

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!

Copy link
Contributor Author

yaiir-a commented Jun 4, 2024
edited
Loading

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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /