Skip to content

Conversation

@mabraham
Copy link
Contributor

@mabraham mabraham commented Oct 9, 2024

Without this, a driver that failed the sanity check could be returned. This leads to attempting to use it when checking URLs, which led to avoidable failures.

Fixes #92

Without this, a driver that failed the sanity check could be returned. This leads to attempting to use it when checking URLs, which led to avoidable failures.

Fixes urlstechie#92
SuperKogito
SuperKogito previously approved these changes Oct 9, 2024
Copy link
Member

@SuperKogito SuperKogito left a comment

Choose a reason for hiding this comment

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

Lgtm 👍

@vsoch
Copy link
Collaborator

vsoch commented Oct 9, 2024

@mabraham if this fixes the error for you, please bump the version in the changelog and up the version in version.py. I'd also prefer the design pattern of having the driver set back to None if the error is triggered instead of having driver and default_driver (two variables) which I find more confusing.

@SuperKogito
Copy link
Member

I'd also prefer the design pattern of having the driver set back to None if the error is triggered instead of having driver and default_driver (two variables) which I find more confusing.

@vsoch Can you explain why? I personally don't have a preference but I find this implementation cleaner.

@vsoch
Copy link
Collaborator

vsoch commented Oct 9, 2024

@vsoch Can you explain why? I personally don't have a preference but I find this implementation cleaner.

It's simple - as a user developer reading the code I only need to track one instead of two variables.

@mabraham
Copy link
Contributor Author

@mabraham if this fixes the error for you, please bump the version in the changelog and up the version in version.py. I'd also prefer the design pattern of having the driver set back to None if the error is triggered instead of having driver and default_driver (two variables) which I find more confusing.

All done!

@vsoch vsoch merged commit 11dd54a into urlstechie:master Oct 10, 2024
4 checks passed
@vsoch
Copy link
Collaborator

vsoch commented Oct 10, 2024

Thank you @mabraham!

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.

Web driver is not installed, but seems to be required

3 participants