manuelroemer / Nullable

A source code only package which allows you to use .NET's nullable attributes in older target frameworks like .NET Standard 2.0 or the "old" .NET Framework.
https://www.nuget.org/packages/Nullable
MIT License
185 stars 8 forks source link

Nullable's XML documentation leaks into XML docs of referencing libraries #8

Closed rdeago closed 4 years ago

rdeago commented 4 years ago

Every library that references Nullable and has XML documentation finds the latter "polluted" by the documentation for Nullable's attributes.

Although I understand that Intellisense is nice to have, I'd rather push F1 when I need an explanation on some attribute than have the attributes' documentation injected into every NuGet package I produce. Not to mention automatically-generated online docs!

I propose that XML documentation be removed completely from Nullable, as the inconveniences it brings to referencing libraries outweigh its usefulness.

manuelroemer commented 4 years ago

Hi, I totally agree, the documentation should not leak. I will look into this.

I‘m thinking about a potential middle ground right now which might allow to preserve the XML documentation during development: The XML doc could simply be wrapped in an #if DEBUG block. Then it would be visible during development, but would not appear in a release build (which should be used for every published library). Would this satisfy your use case?

And thanks for raising the issue! Much appreciated!

rdeago commented 4 years ago

Looks like a good idea to me. Thanks for your quick answer!

If you don't have time to implement the change I can make a PR in a few hours.

manuelroemer commented 4 years ago

I should be able to make the change today when I arrive at home, but thanks for the offer! I will notify you when it is done.

manuelroemer commented 4 years ago

I just uploaded the new version 1.2.1 which should fix the issue as discussed. In DEBUG builds, the XML documentation is still there for convenient development, but in RELEASE builds, it will be omitted and therefore not leak.

I have done my best to test the new version locally with a variety of project types, but I cannot cover every single kind of setup. Please feel free to tell me if the issue persists despite the changes.

rdeago commented 4 years ago

I can confirm that the issue has been resolved. Thanks!

AArnott commented 4 years ago

Not to mention automatically-generated online docs!

What doc generator are you using that produces documentation for internal types from your xml file?

rdeago commented 4 years ago

What doc generator are you using that produces documentation for internal types from your xml file?

Point taken. My main concern, however, was with the extra XML in all NuGet packages for my .NET Standard 2.0 libraries. It just shouldn't have been there. As for doc generators, I may have been mistaken.

AArnott commented 4 years ago

There's probably a lot of xml documentation for internal types in that file for any library that bothers to document itself. I've never heard a complaint myself. And this is just a small amount, I can't imagine why it would matter.

But the work is done to suppress it already, so water under the bridge.