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

Remove inheritdoc #6246

Merged
merged 1 commit into from
Mar 13, 2021
Merged

Remove inheritdoc #6246

merged 1 commit into from
Mar 13, 2021

Conversation

gewarren
Copy link
Contributor

@gewarren gewarren commented Mar 12, 2021

This property has both summary comments and inheritdoc. It causes a docs build warning to have both.

Contributes to dotnet/dotnet-api-docs#5411.

@gewarren gewarren requested a review from jeffkl March 12, 2021 00:45
@@ -113,7 +113,6 @@ public sealed class RoslynCodeTaskFactory : ITaskFactory
/// <inheritdoc cref="ITaskFactory.FactoryName"/>
public string FactoryName => "Roslyn Code Task Factory";

/// <inheritdoc />
Copy link
Member

Choose a reason for hiding this comment

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

📝 This was inheriting from

/// <summary>
/// Gets the type of the task this factory will instantiate. Implementations must return a value for this property.
/// </summary>
Type TaskType { get; }

While it's perfectly valid to include both <inheritdoc/> and other content, it only has an impact when the inherited documentation contains elements that are not manually replaced (e.g. if the inherited documentation had <remarks> or <value>, but this property only contained <summary>.

@sharwell
Copy link
Member

@gewarren Can you verify that this:

It causes a docs build warning to have both.

Only applies to cases where the additional documentation fully replaces the inherited documentation? Partial replacement is a proper supported use case for <inheritdoc/>.

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Mar 12, 2021
@gewarren
Copy link
Contributor Author

@gewarren Can you verify that this:

It causes a docs build warning to have both.

Only applies to cases where the additional documentation fully replaces the inherited documentation? Partial replacement is a proper supported use case for <inheritdoc/>.

Apparently it is not implemented that way in the docs build system. There is no partial replacement of doc comments through inheritdoc. cc @mimisasouvanh

@gewarren
Copy link
Contributor Author

I see that IntelliSense supports partial inheritance.

image

@benvillalobos benvillalobos merged commit 506d46b into master Mar 13, 2021
@benvillalobos benvillalobos deleted the gewarren-patch-1 branch March 13, 2021 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants