-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add legacy workspace eventing APIs to the banned list inside roslyn #78370
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
Add legacy workspace eventing APIs to the banned list inside roslyn #78370
Conversation
eng/config/BannedSymbols.txt
Outdated
| M:Microsoft.CodeAnalysis.DesktopStrongNameProvider.#ctor(System.Collections.Immutable.ImmutableArray{System.String}); Use overload that takes in temp directory | ||
| M:System.Diagnostics.Tracing.EventSource.WriteEvent(System.Int32,System.Object[]); Use WriteEventCore instead | ||
| M:System.Diagnostics.Tracing.EventSource.WriteEvent(System.Int32,System.Object[]); Use WriteEventCore instead | ||
| E:Microsoft.CodeAnalysis.Workspace.WorkspaceChanged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding the comment explaining what to use instead; even if we know it's still helpful to be self-documenting why these lines are here.
| /// Sets the <see cref="CurrentSolution"/> of this workspace. This method does not raise a <see | ||
| /// cref="WorkspaceChanged"/> event. This method should be used <em>sparingly</em>. As much as possible, | ||
| /// derived types should use the SetCurrentSolution overloads that take a transformation. | ||
| /// cref="WorkspaceEventType.WorkspaceChange"/> event. This method should be used <em>sparingly</em>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may need to suppress the pragma here -- I'm not sure if this will trip up any documentation generators since this is now going to an internal type instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bah, the cref isn't that important. The comment is pretty clear without it.
No description provided.