-
Notifications
You must be signed in to change notification settings - Fork 37
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
Enable nullable reference types #590
base: main
Are you sure you want to change the base?
Conversation
Select(featureFlagFilter!, labelFilter!); | ||
|
||
_multiKeyWatchers.AppendUnique(new KeyValueWatcher | ||
{ | ||
Key = featureFlagFilter, | ||
Label = labelFilter, | ||
Key = featureFlagFilter!, | ||
Label = labelFilter!, |
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.
Here there's no check that featureFlagSelector.KeyFilter
and featureFlagSelector.LabelFilter
are not null. They might be since featureFlagSelector
could be initialized with SnapshotName
@@ -1071,11 +1079,11 @@ private async Task<T> ExecuteWithFailOverPolicyAsync<T>( | |||
} | |||
} | |||
|
|||
Uri currentEndpoint = _configClientManager.GetEndpointForClient(clientEnumerator.Current); | |||
Uri currentEndpoint = _configClientManager.GetEndpointForClient(clientEnumerator.Current)!; |
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.
IConfigurationClientManager.GetEndpointForClient
can return null
if it doesn't find the client, but everywhere it's used return value is never checked for null (here's an example)
@@ -54,20 +56,22 @@ private void VisitJsonElement(JsonElement element) | |||
case JsonValueKind.True: | |||
case JsonValueKind.False: | |||
case JsonValueKind.Null: | |||
_data.Add(new KeyValuePair<string, string>(_currentPath, element.ToString())); | |||
_data.Add(new KeyValuePair<string, string?>(_currentPath!, element.ToString())); |
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.
_currentPath
can be null
here if we never execute EnterContext
or ExitContext
@@ -275,7 +276,7 @@ private async Task RefreshFallbackClients(CancellationToken cancellationToken) | |||
var targetEndpoint = new Uri($"https://{host}"); | |||
|
|||
var configClient = _credential == null | |||
? new ConfigurationClient(ConnectionStringUtils.Build(targetEndpoint, _id, _secret), _clientOptions) | |||
? new ConfigurationClient(ConnectionStringUtils.Build(targetEndpoint, _id!, _secret!), _clientOptions) |
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.
If ConfigurationClientManager
is initialized using the constructor with endpoints
, _id
and _secret
will be null here
Fixes #300
Notes:
KeyValueSelector
can ether be initialized with key and label, or snapshot so I introduced two constructors to reflect that