Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Aug 5, 2020

This uses three features of fsdocs to improve the doc content for FSharp.Core.xml.

This give much better documentation for FSharp.Core, e.g. at https://github.com/fsharp/fsharp-core-docs and https://fsharp.github.io/fsharp-core-docs/

The features are:

  1. ordered categories <category> and <categoryindex>

  2. exclusion using <exclude /> (note, the same extension to XML docs is used by sandcastle)

  3. documentation for namespaces <namespacesummary> and <namespaceremarks>

See fsprojects/FSharp.Formatting#580 where the support for these features is added

Note that the C# XML doc specification explicitly says it's ok to use extensions to XML docs to support other documentation generation systems. And that is of course the "X" in "XML". From https://docs.microsoft.com/en-us/dotnet/csharp/codedoc:

All the tags outlined above represent those that are recognized by the C# compiler. However, a user is free to define their own tags. Tools like Sandcastle bring support for extra tags like and , and even support documenting namespaces. Custom or in-house documentation generation tools can also be used with the standard tags and multiple output formats from HTML to PDF can be supported.

Given the importance of good quality FSharp.Core doc generation I think it's reasonable for us to add this extra information into the FSharp.Core XML docs.

@cartermp
Copy link
Contributor

cartermp commented Aug 5, 2020

I don't want to move forward on any of this until we can come to a consensus on these kinds of PRs. The more this branch moves and master does not, the harder it will be to actually incorporate it (for example - for F# 5). I would much rather that these changes just go into master and flow through our normal shipping channels so that when an FSharp.Core 5.0 is cut it will simply have these improvements

@dsyme
Copy link
Contributor Author

dsyme commented Aug 6, 2020

I don't want to move forward on any of this until we can come to a consensus on these kinds of PRs. The more this branch moves and master does not...

I re-read this comment and I think there's a mis-understanding here. This PR pushes doc changes to master, just like you're asking for.

I think your concern is with having the "feature/docs" branch (which I'm pushing from) as a head branch that co-integrates with master. I do understand your concern with that (though I think it's not a big deal), but either way it's a different issue to actually pushing doc improvements to master, which is what's happening here.

@cartermp
Copy link
Contributor

cartermp commented Aug 6, 2020

Sorry, I wasn't paying attention to the target branch here. I'm highly allergic to more of these branches sitting around, especially with the added bot traffic. It creates so much noise that anyone other than MS employees are strongly discouraged from following the repo

@dsyme
Copy link
Contributor Author

dsyme commented Aug 6, 2020

Sorry, I wasn't paying attention to the target branch here. I'm highly allergic to more of these branches sitting around, especially with the added bot traffic. It creates so much noise that anyone other than MS employees are strongly discouraged from following the repo

Yes, I can see the problem. I'd perfectly fine with having feature branches out of repo if only we could easily get best CI and (for docs) automatic integration PRs set up across repos.

I've added https://github.com/fsharp/fslang-design/blob/master/tooling/FST-1031-xmldoc-extensions.md to document the tags used here, which I'd like you to double check please. In the process of writing the RFC I adjusted the tags, I'll update the FSharp.Formatting implementation and this PR now to correspond.

@abelbraaksma
Copy link
Contributor

It creates so much noise that anyone other than MS employees are strongly discouraged from following the repo

It indeed clutters the notifications, and I click a lot of it away each day (not such a big deal, but still). There's currently no way to tell github that I'm not interested in bot updates. Maybe this could be requested to the github team? (unf they don't have a public repo themselves, nor a public bug tracking system afaik).

@auduchinok
Copy link
Member

I'd perfectly fine with having feature branches out of repo if only we could easily get best CI and (for docs) automatic integration PRs set up across repos.

This would be really great!

@cartermp
Copy link
Contributor

cartermp commented Aug 6, 2020

Maybe this could be requested to the github team?

If only they responded to feature requests coming internally from MS, we wouldn't have had this issue for a long time now :(

I'd perfectly fine with having feature branches out of repo if only we could easily get best CI and (for docs) automatic integration PRs set up across repos.

If it requires regularly syncing back into this one then it's just rearranging where the bot PRs come from. If it's only in one direction - dotnet/fsharp --> something/something - then that's probably fine.

@KevinRansom KevinRansom merged commit 4e2e51c into master Aug 7, 2020
@dsyme
Copy link
Contributor Author

dsyme commented Aug 7, 2020

Thanks for merging this, much appreciated


[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
/// <summary>Basic operations for values of type <see cref="T:Microsoft.FSharp.Core.Result`2"/>.</summary>
/// <summary>Contains operations for working with values of type <see cref="T:Microsoft.FSharp.Core.Result`2"/>.</summary>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this and others leads to a 404 in the docs page

nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* various doc improvements

* don't use uppercase since in cref it is shown as text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants