Made the port optional #239
Conversation
|
|
✅ Build minestat 1.0.0.465 completed (commit 72fbbfe077 by @smellilac) |
There was a problem hiding this comment.
Thank you for the contribution, @smellilac. Aside from my review questions, the only other issue I see is with indentation. Our code style uses 2 spaces.
I am also including @Ajoro to see if he has any additional suggestions.
| #elif NET46 | ||
| using ARSoft.Tools.Net; | ||
| using ARSoft.Tools.Net.Dns; | ||
| #endif |
There was a problem hiding this comment.
Would we want to handle all other cases with an #else here?
There was a problem hiding this comment.
I don't see how #else can be used logically here. The only thing that comes to mind is a construction like this:
#else
#error This target framework is not supported by this code. Please ensure the target is .NET Standard 2.0 or greater, or .NET Framework 4.6 or greater.
| var srvRecord = srvRecords.First(); | ||
| return (ushort)srvRecord.Port; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Are there any conditions that should be covered under an #else?
There was a problem hiding this comment.
In this case I would not use #else, as I already suggested adding it above. That way, if the wrong framework is used. The compiler will not even get to this code.
ldilley
added
enhancement
|
Closing due to lack of response from author. |
Fixes #238