-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
[dotnet] Unify protected and internal Execute methods #15233
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. I think this PR is still actual, so just actualize it and move forward. Thank you!
Test failures are unrelated: //dotnet/test/common:BiDi/Network/NetworkEventsTest-firefox
//java/test/org/openqa/selenium/mobile:NetworkConnectionTest
//java/test/org/openqa/selenium/remote:RemoteWebDriverScreenshotTest
//java/test/org/openqa/selenium/remote:RemoteWebDriverScreenshotTest-firefox-beta
//rb/spec/integration/selenium/webdriver:network-firefox-beta-bidi
//rb/spec/integration/selenium/webdriver:network-firefox-bidi |
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
protected
+internal
->protected internal
Motivation and Context
Less overloads, shorter stack traces, less maintenance.
Types of changes
Checklist
PR Type
Enhancement, Bug fix
Description
Unified
InternalExecute
andExecute
methods into a singleprotected internal
method.Replaced all occurrences of
InternalExecute
withExecute
across multiple classes.Added null checks for constructor parameters in
HttpCommandInfo
.Simplified and streamlined method calls for better maintainability and reduced stack traces.
Changes walkthrough 📝
10 files
Replaced `InternalExecute` with `Execute` in alert handling.
Replaced `InternalExecute` with `Execute` in cookie management.
Replaced `InternalExecute` with `Execute` in log retrieval.
Replaced
InternalExecuteAsync
withExecuteAsync
in navigation methods.Replaced
InternalExecute
withExecute
in shadow DOM element handling.Replaced
InternalExecute
withExecute
in frame and window switching.Replaced `InternalExecute` with `Execute` in timeout management.
Unified
InternalExecute
andExecute
methods into a singleprotected
internal
method.Replaced
InternalExecute
withExecute
in element-specific commands.Replaced `InternalExecute` with `Execute` in window management.
1 files
Added null checks for constructor parameters.