Skip to content
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

Support additional browser backends + refactoring #4

Conversation

rb2k
Copy link
Contributor

@rb2k rb2k commented Nov 23, 2011

This is a "large" change and some of the refactorings might be somewhat opinionated.
This allows people to test on non-rails projects (e.g. when testing external webapps) using the selenium/capybara-webkit/poltergeist drivers.

I hope I didn't break any of the non-cucumber or rails related things.

Cheers,
Marc

@mattheworiordan
Copy link
Owner

Wow, thanks for putting in the time for this gem. I will go through the code when I have a chance and do some testing before I merge the pull request, but unless I find any issues, I will definitely incorporate.

Thanks again.

@@ -1,22 +1,51 @@
module Capybara
module Screenshot
class Saver
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this away from a class to a module. Since we don't have any internal state, there is no need to use a class :)

Copy link
Owner

Choose a reason for hiding this comment

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

Fair enough

@rb2k
Copy link
Contributor Author

rb2k commented Nov 23, 2011

Added some comments to make the changes more clear.

mattheworiordan pushed a commit that referenced this pull request Nov 24, 2011
Support additional browser backends + refactoring
@mattheworiordan mattheworiordan merged commit 8bba963 into mattheworiordan:master Nov 24, 2011
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