Add support for nullable types - static headers#7852
Conversation
| /// <param name="config">The <see cref="EndpointConfiguration" /> instance to apply the settings to.</param> | ||
| /// <param name="key">The static header key.</param> | ||
| /// <param name="value">The static header value.</param> | ||
| public static void AddHeaderToAllOutgoingMessages(this EndpointConfiguration config, string key, string value) |
There was a problem hiding this comment.
This being a public API means some more consideration is needed here.
Before this PR, because the only argument validation that is happen is related to key, that means that null is valid for for value.
Adding #nullable enable here is changing what the code is describing as valid.
Should null be allowed, or should be preventing it? What about breaking change concerns?
These are the sort of things that need to be considered when enabling nullable annotations, especially on public APIs.
soujay
changed the title
| public static void AddHeaderToAllOutgoingMessages(this EndpointConfiguration config, string key, string value) | ||
| { | ||
| ArgumentException.ThrowIfNullOrWhiteSpace(key); | ||
| ArgumentException.ThrowIfNullOrWhiteSpace(value); |
There was a problem hiding this comment.
This matches with CurrentStaticHeaders, which is defined as Dictionary<string, string>
Also, since we're already validating the key with the same method, it makes sense to apply the same rigor to the value.
4 participants
This PR enables
#nullable enableon the files in the folder, and removesNServiceBus.StaticHeadersConfigExtensionsfromNullableAnnotations.ApproveNullableTypes.approved.txt