Mayvenn / limo

A wrapper around selenium webdriver
Eclipse Public License 1.0
25 stars 4 forks source link

Add more support to by/locator and use recent version of selenium #1

Closed agilecreativity closed 7 years ago

agilecreativity commented 7 years ago

Hi, Thanks for the great library. This PR include the following changes:

If you like me to adjust anything please let me know. Cheers, Burin

sheelc commented 7 years ago

Thanks for this PR Burin! It looks pretty good to me, we'll see if @jeffh has any feedback.

In the meantime, I noticed what could be a small issue on the :css fallback that I've added a comment/review for.

agilecreativity commented 7 years ago

Hi @sheelc,

Thanks for your feedback and quick turn around. I updated the code based on your comment and also add new test to cover this case. I can see the test are passing locally. As we don't have the CI/CD I was not not aware of the issue until your report.

Hopefully this is ok.

Looking forward to get some feedback from @jeffh Cheers, Burin

sheelc commented 7 years ago

Perfect, thanks Burin. I just enabled CircleCI for PR/forks of this repo so that there's some test feedback. Hopefully this comment triggers a build, otherwise it looks like there's some other actions to try to get CircleCI to kick in

sheelc commented 7 years ago

Aw, looks like it didn't do it. I guess it only works with new commits. That's okay though, it works for me locally and CI will run when merged to master. Thanks for all the work on this Burin!

jeffh commented 7 years ago

Thanks @agilecreativity!