Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Import XML Doc Comments for System.ComponentModel.EventBasedAsync #8282

Conversation

cartermp
Copy link

@cartermp cartermp commented May 4, 2016

@mairaw
Copy link

mairaw commented May 4, 2016

In this case where we have the same classes in both files and no extra APIs, should we keep the comments in just one of the files?

@cartermp
Copy link
Author

cartermp commented May 4, 2016

Good question. In this case one of the partial BackgroundWorker classes does implement IDisposable, so there is some significance. I don't think it's too bad to have the same <summary> for both of them in this case.

@mairaw
Copy link

mairaw commented May 4, 2016

That's fine. But I'm thinking of how docfx detects the content and what if we need to make edits? Do we have to update it in two places? It becomes a maintenance nightmare.

@cartermp
Copy link
Author

cartermp commented May 4, 2016

That's true. I see options here:

  1. Merge the definition in the Manual.cs file with the other one.
  2. Remove the <summary> comment in the Manual.cs file.

The second option is certainly easier.

@mairaw
Copy link

mairaw commented May 4, 2016

In this case, the #2 option seems to work since you have everything in the other file. That distinction is not that clear for cases like System.Reflection.Emit where the APIs are literally broken into the 2 files.

@cartermp cartermp force-pushed the import-sys-componentmodel-eventbasedasync branch from 65b52dc to adf78e7 Compare May 5, 2016 17:35
@cartermp
Copy link
Author

cartermp commented May 5, 2016

Done.

@cartermp
Copy link
Author

cartermp commented May 9, 2016

Closing this out as we have a different plan for reference docs now.

@cartermp cartermp closed this May 9, 2016
@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants