search

samnissen / watir-screenshot-stitch

Extends Watir to take full page screenshots of websites.
MIT License
17 stars 7 forks source link

remove the need to pass in browser as parameter to the methods #31

Closed titusfortner closed 6 years ago

titusfortner commented 6 years ago

The goal is to get rid of the need to pass in a Browser instance.

The first question is if we need to maintain backward compatibility, or if we're ok just having everyone who wants to update have to change their code.

The second question is whether you want to override what is in Watir now, or to wait for https://github.com/watir/watir/issues/731 to get added and Watir 6.12 released.

This PR shows how you can update the code right now in a way that is not backward compatible. Assuming Watir 6.12 includes the code from that PR, you would only need to remove the Browser#screenshot and Screenshot#initialize methods and force the version in the gemfile.

samnissen commented 6 years ago

Thanks for this! Tested this and it passes here.

It makes sense to me to wait until Watir 6.12, as I don't view this as especially urgent vs. the drop in complexity it would provide. I'm not against a breaking change, though -- I could release a gem version with a warning of the coming change in the interim.

I'm happy to allow you to handle the code changes to this PR to require 6.12 and remove the corresponding methods. However, if you prefer I handle it, please just say.

titusfortner commented 6 years ago

Yup, sounds good. Hoping for a new Watir release next week, but we'll see.