Skip to content

Conversation

@aik-jahoda
Copy link
Contributor

Summary

Collect all changes from #3916 related to NegotiateStream

@dotnet-bot dotnet-bot added this to the June 2020 milestone Jun 4, 2020
@karelz karelz requested review from davidsh and wfurt June 9, 2020 00:29
@davidsh davidsh requested a review from gewarren June 9, 2020 00:35
## Examples
The following code example demonstrates flushing the stream.
[!code-csharp[NclNegoSyncClient#4](~/samples/snippets/csharp/VS_Snippets_Remoting/NclNegoasyncClient/CS/client.cs#4)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this section in the file is referred from multiple places. I will take a look if it renders correctly on other places too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is quite inconsistency in the examples:
C++ and VB are same and uses BexinXXX, EndXXX methods
C# uses XXXAsync methods.

NclNegoasyncClient.cpp
client.cs
client.vb

It implies that in the BexinXXX method docs we show C# example however not for the specific api. On other side C++ and VB are accurate.

I'm not sure how to handle it as there is several options:

  1. Create BexinXXX, EndXXX variant for C# async methods
    1. it demonstrate legacy approach
    2. we still need Task based async examples - code duplication
  2. Let it like it is and remove C# example from BexinXXX, EndXXX
    1. It sounds strange to me as C# is main .net language and we would offer examples in other languages only
  3. Rework VB + C++ to not use BexinXXX, EndXXX but Task based async methods
    1. It would unify the examples
    2. it would require remove examples from BexinXXX, EndXXX docs and move it to Task based async methods

Example of mismatched examples:
https://docs.microsoft.com/en-us/dotnet/api/system.net.security.negotiatestream.beginwrite?view=netcore-3.1

The C# example doesn't contain the related api.

@karelz, any thought?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the example completely. There is follow up ticket: #4390

aik-jahoda and others added 3 commits June 11, 2020 11:20
Co-authored-by: Genevieve Warren <gewarren@microsoft.com>
@aik-jahoda aik-jahoda requested a review from gewarren June 16, 2020 09:34
@aik-jahoda aik-jahoda merged commit 892ea13 into dotnet:master Jun 17, 2020
@aik-jahoda aik-jahoda deleted the jajahoda/NegotiateStream branch June 17, 2020 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants