Fixed various typos in comments and field names. Fixed Exception for passing comment correctly #286
Conversation
CycoPH
commented
Sep 23, 2019
CycoPH
commented
- Fixed various typos in comments
- Fixed the naming of a field. ArrivalSubscription had a typo
- Fixed bug in NATSBadSubscriptionException.
- The passed error message was never forwarded to the NATSException container
- The passed error message was never forwarded to the NATSException container
ArrivalSubscription had a type
| @@ -162,7 +162,7 @@ public void AssignData(byte[] data) | |||
| /// <summary> | |||
| /// Gets the <see cref="ISubscription"/> which received the message. | |||
| /// </summary> | |||
| public ISubscription ArrivalSubcription | |||
| public ISubscription ArrivalSubscription | |||
There was a problem hiding this comment.
This is a breaking change to the API (but a very reasonable one). Perhaps we should consider the original misnamed property forwarding to the new property, with the original property marked as Obsolete?
There was a problem hiding this comment.
Yes, I agree. Wow, am I embarrassed at that one. Yep, we should keep both, and mark ArrivalSubcription obsolete. We can then remove it for version 1.0.
There was a problem hiding this comment.
@CycoPH, would you mind adding a new property (correcting my spelling), adding an obsolete attribute, e.g.
[ObsoleteAttribute("This property will soon be deprecated. Use ArrivalSubscription instead.")]
...and have the old property call the new one?
There was a problem hiding this comment.
Sure did the change a resubmitted
|
LGTM, other than the comments above. Thank you for the contribution! |
| /// Gets the <see cref="ISubscription"/> which received the message. | ||
| /// </summary> | ||
| [ObsoleteAttribute("This property will soon be deprecated. Use ArrivalSubscription instead.")] | ||
| public ISubscription ArrivalSubcription |
There was a problem hiding this comment.
Should be able to make this a one liner:
public ISubscription ArrivalSubcription => ArrivalSubscription;
