-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
[dotnet] Fully annotate Command
for AOT support
#15527
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
base: trunk
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
@@ -31,24 +32,33 @@ namespace OpenQA.Selenium | |||
/// </summary> | |||
public class Command |
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.
Best viewed with whitespace off
User description
AOT warnings are user-facing, even if those warnings are within Selenium's own code. For that reason, we should ensure we are not emitting any warnings unless such code paths are annotated as AOT-unsafe.
Motivation and Context
Contributes to #14480
Types of changes
Checklist
PR Type
Enhancement
Description
Added AOT-friendly annotations to the
Command
class.Refactored
JsonSerializerOptions
into a static nested class for safety.Introduced helper methods for parameter handling in
Command
.Updated
HttpCommandInfo
to use new parameter handling methods.Changes walkthrough 📝
Command.cs
Refactor `Command` class for AOT compatibility
dotnet/src/webdriver/Command.cs
JsonSerializerOptions
into a static nested class.HasParameters
andTryGetValueAndRemoveIfNotNull
helpermethods.
HttpCommandInfo.cs
Update parameter handling in `HttpCommandInfo`
dotnet/src/webdriver/HttpCommandInfo.cs
HasParameters
andTryGetValueAndRemoveIfNotNull
.