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.

Sign up for GitHub

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

Jump to bottom

[rb] have Selenium Manager binary locate drivers on PATH #12345

Merged
merged 1 commit into from
Jul 13, 2023
Merged

[rb] have Selenium Manager binary locate drivers on PATH #12345

merged 1 commit into from
Jul 13, 2023

Conversation

titusfortner
Copy link
Member

Description

  • Remove the use of Platform#find_binary in DriverFinder
  • Remove #find_binary from Platform
  • Fixed unit tests

Motivation and Context

Considerations

  • It is possible for this to be a breaking change since the find_binary logic may differ slightly from the binary, but it is unlikely and warnings will be thrown in the console.
  • Are there any of these methods in this API private class that I'm deleting that we want to keep?

@titusfortner titusfortner requested a review from p0deje July 12, 2023 03:35
@titusfortner titusfortner mentioned this pull request Jul 12, 2023
Closed
p0deje
p0deje approved these changes Jul 12, 2023
View reviewed changes
Copy link
Member

@p0deje p0deje left a comment

Choose a reason for hiding this comment

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

Looks great!

@titusfortner titusfortner merged commit 054467a into trunk Jul 13, 2023
@titusfortner titusfortner deleted the rb_path branch July 13, 2023 13:32
@gaffneyc gaffneyc mentioned this pull request Aug 7, 2023
Closed
@zarembas zarembas mentioned this pull request Aug 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p0deje p0deje p0deje approved these changes

Assignees
No one assigned
Labels
C-rb
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@titusfortner @p0deje