brudo commented on PR #23: URL: https://github.com/apache/activemq-nms-openwire/pull/23#issuecomment-1166056469
Reviewing the change in PR #21, I had not changed the fallback return value in GetAllowedProtocol from Default to None; I only had the change to the default constructor. In case we are squashing and merging this PR instead, I made a similar, but not identical change here for consideration. One functional difference between the two implementations - I used Enum.TryParse instead of checking the string value before calling Parse. That way it will fall back to None (0) and not raise an exception even if an invalid string (one not found in the enum) was used in the SslContext. I think this is an improvement; others may disagree... To my way of thinking, if we want to raise an exception for invalid values, it would be better to have it validate the input in the SslContext(string) constructor instead of leaving it to be thrown later, when GetAllowedProtocol is called from CreateSocketStream. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
