NotificationHubConnectionStringBuilder has no ToString method about azure-notificationhubs-dotnet HOT 5 CLOSED

@StevenLiekens thanks for the bug report. You're correct that this should just work. I'll add this to our backlog. We also accept pull requests 😉 .

from azure-notificationhubs-dotnet.

Looks like this was completed by #91. I'm closing out this issue.

from azure-notificationhubs-dotnet.

Hey @brannon, thanks for the reply. Given that I want to implement the ToString method and submit a PR, what tests do you expect me to write? I'm not sure how this class should handle garbage values like SharedAccessKeyName = null or "" or "🗑️". I'm also a little confused about the test project layout. Should the test go inside the DotNetCore.Tests project and then be linked from the DotNetFramework.Tests project?

from azure-notificationhubs-dotnet.

I'm not @brannon, but my $0.02 on this:

I'm not sure how this class should handle garbage values like SharedAccessKeyName = null or "" or "🗑️".

I would not have the ToString be responsible for flagging garbage values. Rather, I would just go ahead and print out the results of those properties as stored in the properties. If we want/need validation in the future, it should live in the property setters.

Should the test go inside the DotNetCore.Tests project and then be linked from the DotNetFramework.Tests project?

I believe you have this right. Looking at the DotNetFramework.Tests project, the only items that are included in it are actually just linked in from the DotNetCore project. Following that pattern makes sense to me.

from azure-notificationhubs-dotnet.

Thanks @marstr for chiming in. I agree that ToString should just format the property values as set in the properties. No need to validate the properties at this time.

The idea with the test projects is to provide a common set of tests that we can run for both .NET Core and .NET Framework targets.

from azure-notificationhubs-dotnet.