Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML docs for LINQ Join, LeftJoin, RightJoin #111177

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

roji
Copy link
Member

@roji roji commented Jan 7, 2025

Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-linq
See info in area-owners.md if you want to be subscribed.

@@ -178,6 +178,104 @@ private static Expression GetSourceExpression<TSource>(IEnumerable<TSource> sour
return q != null ? q.Expression : Expression.Constant(source, typeof(IEnumerable<TSource>));
}

/// <summary>
/// Correlates the elements of two sequences based on matching keys.
/// The default equality comparer is used to compare keys.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: try to keep summary text concise and restricted to one sentence. The part about using the default equality comparer could be moved to the remarks section.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the idea was to already make clear the difference between Join and LeftJoin/RightJoin right at the summary - but will remove.

/// The following code example demonstrates how to use <see cref="Join{TOuter, TInner, TKey, TResult}(IQueryable{TOuter}, IEnumerable{TInner}, Expression{Func{TOuter, TKey}}, Expression{Func{TInner, TKey}}, Expression{Func{TOuter, TInner, TResult}})" /> to perform an inner join of two sequences based on a common key.
/// </para>
/// <code>
/// class Person
Copy link
Member

Choose a reason for hiding this comment

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

@carlossanlop @gewarren I don't think I've seen us use <code> blocks in dotnet/runtime source code before. Does the porting tool handle them correctly or should the code samples by added to dotnet-api-docs in a different way?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should work. Even better is to add a language tag: https://learn.microsoft.com/en-us/contribute/content/dotnet/api-documentation#language-attributes.

You can also consider putting these examples in separate files since they take up quite a few lines. I'm not exactly sure on the plumbing, but the dotnet/machinelearning repo uses separate code files: https://github.com/dotnet/machinelearning/blob/886e2ff125c0060f5a251056c7eb2a7d28738984/src/Microsoft.ML.TensorFlow/TensorflowCatalog.cs#L32.

The code files actually live in the dotnet/machinelearning repo too: https://github.com/dotnet/machinelearning/blob/main/docs/samples/Microsoft.ML.Samples/Dynamic/TensorFlow/TextClassification.cs. But I believe the URL looks the way it does (~/../docs/samples/docs/samples/Microsoft.ML.Samples/Dynamic/TensorFlow/TextClassification.cs) to make it work on learn.microsoft.com, and I'm not sure if this would work in IntelliSense, or if that even matters since I don't think examples are shown. The docs repo hooks up the URL root /docs/samples here.

Lengthy remarks can also go in separate files. You can see an example of that in this repo.

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.

3 participants