Skip to content

favor chromedriver over phantomjs#17

Open
mattwr18 wants to merge 17 commits intomasterfrom
16-favor-chromedriver-over-phantomjs
Open

favor chromedriver over phantomjs#17
mattwr18 wants to merge 17 commits intomasterfrom
16-favor-chromedriver-over-phantomjs

Conversation

@mattwr18
Copy link
Copy Markdown

fixes#16

Copy link
Copy Markdown
Member

@FedericoEsparza FedericoEsparza left a comment

Choose a reason for hiding this comment

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

@mattwr18 Shall I merge this?

@mattwr18
Copy link
Copy Markdown
Author

mattwr18 commented Mar 14, 2019

oh hey @FedericoEsparza sorry, I meant to reach out to you and see if you wouldn't mind merging some PR for us... I know you're busy, but if feels weird making changes to master

This is ready to be merged, I believe, we have left the c9 code just in case we want to go back to using phantomjs

Copy link
Copy Markdown
Member

@FedericoEsparza FedericoEsparza left a comment

Choose a reason for hiding this comment

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

I don't know that I can merge this.

It had a lot of issues on my computer (running phx 1.4)

I pushed up a branch PR-review-branch-16-

I made several error-driven updates in pushing to simply run mix test with chromedriver. (i.e. chromedriver & mix test && pkill chromedriver)

I didn't have enough time to get it to work.

I got as far as a BEAM (OTP compiled version) issue:
Screen Shot 2019-03-20 at 7 24 49 AM

You're welcome to pull down the branch and troubleshoot.

Then you can use the above link to put in a PR to the original PR.

OR if it's working on your machines, feel free to just merge the original PR

Sorry for not reviewing this sooner.

@mattwr18
Copy link
Copy Markdown
Author

sorry for the late reply, this highlights the issue with getting it working in one environment and not properly testing it in others... really appreciate your attention to this @FedericoEsparza

- Update hound in deps to use https instead of ssh for github
- delete order.tests
@mattwr18
Copy link
Copy Markdown
Author

it's working for @aonomike on Mac, we haven't tried it with Phoneix 1.4 because we are on 1.3.
sorry for the ignorance, but I'm a bit confused when you say you are "issues on my computer (running phx 1.4)", I thought this was project based not computer based 🤔

We can certainly merge this, if you prefer, don't want cause extra work for you, but we are also not in a huge rush, maybe we'll go forward in the book just next week.

@aonomike will put in a PR to set up the postgres info as environment variables. 😸

@mattwr18 mattwr18 force-pushed the 16-favor-chromedriver-over-phantomjs branch from 9309dd5 to 7a658a2 Compare April 24, 2019 14:10
- issue with jdk version preinstalled?

Co-authored-by: Mike Aono <aonomike@gmail.com>
@mattwr18 mattwr18 force-pushed the 16-favor-chromedriver-over-phantomjs branch from 7a658a2 to 57a433c Compare April 24, 2019 14:13
mattwr18 and others added 2 commits April 24, 2019 11:21
- 11 not supported by trusty

Co-authored-by: Mike Aono <aonomike@gmail.com>
Co-authored-by: Mike Aono <aonomike@gmail.com>
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.

Replace Phantomjs with Chrome driver

3 participants