jstedfast / MailKit

A cross-platform .NET library for IMAP, POP3, and SMTP.
http://www.mimekit.net
MIT License
6.04k stars 809 forks source link

Code style: use <inheritdoc cref="..."> instead of copying doc #1718

Closed rklec closed 4 months ago

rklec commented 4 months ago

Is your feature request related to a problem? Please describe. E.g. this code snippet here has C# XML documentation: https://github.com/jstedfast/MailKit/blob/ff722f1a0967cb2118d109a5a2136dff2c793ac4/MailKit/Net/Imap/ImapClient.cs#L695-L704

However, I found out it actually implements this interface: https://github.com/jstedfast/MailKit/blob/ff722f1a0967cb2118d109a5a2136dff2c793ac4/MailKit/IMailService.cs#L212-L219

This here also implements it: https://github.com/jstedfast/MailKit/blob/ff722f1a0967cb2118d109a5a2136dff2c793ac4/MailKit/MailService.cs#L296-L305

They all have the same doc, as they implement the same interface etc. This leads to:

Describe the solution you'd like Use <inheritdoc> instead. It allows you to inherit the documentation without duplicating. I found that nowhere to be used in this project.

If the compiler does not know/it's unclear which interface to inherit, you can use cref to specify it. Resharper etc. have great support for this.

Describe alternatives you've considered

Additional context Inheritdoc is really powerful and you can e.g. BTW also inherit just some paths.

Also, by default, you can also inheritdoc and add a tag not in the source, so e.g. add a remarks if some additional information only for this method. And, obviously, if the doc should really be totally different on purpose, then you won't need to use it at one place. Just try that out with IntelliSense.

jstedfast commented 4 months ago

I'd rather not switch to inheritdoc right now.

rklec commented 4 months ago

Is there any reason for that?

jstedfast commented 4 months ago

In the docs for my interfaces, for example, lets say IMailFolder - various docs will reference IMailStore. But in the abstract MailFolder class, those docs will instead reference MailStore instead of IMailStore.

Also, for methods like Connect, I have exception docs that reference the abstract ProtocolException and CommandException types but in ImapClient, they will reference ImapProtocolException and ImapCommandException, respectively.

Yes, I know, technically I can skirt around all that by jumping through hoops with inheritdoc, but it's just not worth the effort for me. It's much easier to c&p the docs.

In many cases, like for properties such as IsSecure, I could probably use inheritdoc because there isn't anything specific per client, but at this point, it's not worth the effort.

rklec commented 4 months ago

In the docs for my interfaces, for example, lets say IMailFolder - various docs will reference IMailStore. But in the abstract MailFolder class, those docs will instead reference MailStore instead of IMailStore.

I can try a PR for that example, but I'd say this is generally possible without any problems. With cref, as said, you can reference IMailStore or, if needed, MailStore or whatever you need. Actually, cref does not need to follow any polymorphism/object-oriented hierarchy or so, you can reference anything that is accessible.

In the end, you would just copy the correct inheritdoc comment. C&P the docs itself has disadvantages as explained, like being affected by (copy) mistakes etc. So kind of, in the longer-term that effort of using it, should pay off, as it should decrease maintenance of all that comments in different places, IMHO.

jstedfast commented 4 months ago

I'm not interested. Just let it be.