-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix UseCompilerGeneratedDocXmlFile default value #106179
Conversation
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.
For Tensors, instead of pragma disables, can we instead disable the property in the csproj? The APIs are actively getting documented, so we should keep it simple and easy to activate, as well as try to avoid a bunch of merge conflicts #106084
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries |
Disabling the property in the project file is probably not what we want. That stops the compiler from emitting CS1591 for new API that is getting added or from validating that existing documented API stays documented. It should be trivial for @michaelgsharp to react to this change in his PR (search for '1591' and remove). If his PR gets merged first, then this PR can be simplified. |
<IsTrimmable>false</IsTrimmable> | ||
<UseCompilerGeneratedDocXmlFile>false</UseCompilerGeneratedDocXmlFile> |
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.
Looks like this is causing an error due to it's use of a PNSE.
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.
LGTM, once we get System.Speech working correctly. Might have to manually override the check on this one to allow for no XML for the PNSE (better than no docs at all)
Fixes what was reported in #105974 (comment) by @michaelgsharp (thanks!)
Regressed with #94234
For now suppress the undocumented public APIs that got added since the property got switched to the wrong default. These public APIs are tracked via #105974.
@carlossanlop @ericstj please take a look and merge