-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Annotate nullability on command repository #14888
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:
|
/// <para>-or-</para> | ||
/// <para>If <paramref name="commandInfo"/> is <see langword="null"/>.</para> | ||
/// </exception> | ||
/// <exception cref="ArgumentException">If <typeparamref name="T"/> is not a valid command type for this repository.</exception> |
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.
ArgumentOutOfRangeException
is better? Just to double-check, I trust you.
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.
Personally I would make this an InvalidOperationException
. Even better, I would make this class generic on TCommandInfo
and pass in that value. (this would also remove the need for the Type RepositoryCommandInfoType
property).
However, both of those are breaking changes. This type is public
and I can only assume someone somewhere may be using this. Since the exception is based on invalid input, ArgumentException
isn't necessarily wrong.
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.
Description
Annotates nullability on the
CommandInfoRepository
class, as well as its sole implementationMotivation and Context
Contributes to #14640
Types of changes
Checklist
PR Type
enhancement
Description
CommandInfoRepository
andW3CWireProtocolCommandInfoRepository
classes.GetCommandInfo
method to useTryGetValue
.W3CWireProtocolCommandInfoRepository
using expression-bodied members.Changes walkthrough 📝
CommandInfoRepository.cs
Annotate nullability and improve method documentation
dotnet/src/webdriver/CommandInfoRepository.cs
TryGetValue
for dictionary access.W3CWireProtocolCommandInfoRepository.cs
Enable nullability and simplify property getters
dotnet/src/webdriver/Remote/W3CWireProtocolCommandInfoRepository.cs