Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[docs] Rethink the documentation workflow #12573

Closed
wants to merge 2 commits into from

Conversation

jonpryor
Copy link
Member

Context: https://github.com/xamarin/Xamarin.Forms-api-docs

Currently, Xamarin.Forms documentation is maintained in a separate git
repo: xamarin/Xamarin.Forms-api-docs. This requires that whenever
new types or members are added to e.g. Xamarin.Forms.Core.dll,
the following steps must be performed:

  1. Make the desired changes within Xamarin.Forms.Core.
  2. Run make docs or make Xamarin.Forms.Core.docs to generate new
    mdoc(5) documentation stubs for Xamarin.Forms.Core.dll.
  3. Write the documentation within the files generated in (2).
  4. Manually merge those changes with the corresponding files from
    Xamarin.Forms-api-docs/docs.

This workflow is complicated, and scary -- particularly step (4).

There is an alternative workflow: Instead of treating
xamarin/Xamarin.Forms-api-docs as the "source of truth" for API
documentation, we can instead use "conventional"
C# documentation comments to be the single source of truth.
C# documentation comments can be "inline" -- as is already done for
several files within this repo already -- or they can be stored in
separate XML files, via the <include/> tag.
See xamarin/Java.Interop@9d6e160c for an example.

When C# documentation comments are used, the C# compiler will emit an
e.g. Xamarin.Forms.Core.xml file as part of the build process.

Xamarin.Forms.Core.xml can be fed to mdoc update --import PATH,
which will cause mdoc update to import the documentation present
within Xamarin.Forms.Core.xml and merge the contents into the
generated mdoc(5) XML files.

Note: mdoc update always prefers imported documentation over
whatever documentation already exists.

For demonstration purposes, I've:

  1. Updated Xamarin.Forms.Core.csproj to generate the
    Xamarin.Forms.Core.xml documentation file.

  2. Updated Xamarin.Forms.Core.csproj to ignore the
    CS1573 and CS1591 warnings, so that it still builds;
    it will take some time to import all existing docs into the
    xamarin/Xamarin.Forms repo, if this is desirable.

  3. Merged the API docs for
    Xamarin.Forms.Internals.ActionSheetArguments from
    xamarin/Xamarin.Forms-api-docs into ActionSheetArguments.cs

  4. Updated the make Xamarin.Forms.Core.docs target to use
    mdoc update --import Xamarin.Forms.Core.xml.

This allows the generated docs/Xamarin.Forms.Core directory to
contain the original C# documentation comments-based documentation.

Description of Change

Issues Resolved

  • fixes #

API Changes

None

Platforms Affected

  • Core/XAML (all platforms)
  • iOS
  • Android
  • UWP

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

Context: https://github.com/xamarin/Xamarin.Forms-api-docs

Currently, Xamarin.Forms documentation is maintained in a separate git
repo: xamarin/Xamarin.Forms-api-docs.  This requires that whenever
new types or members are added to e.g. `Xamarin.Forms.Core.dll`,
the following steps must be performed:

 1. Make the desired changes within Xamarin.Forms.Core.
 2. Run `make docs` or `make Xamarin.Forms.Core.docs` to generate new
    [**mdoc**(5)][0] documentation stubs for `Xamarin.Forms.Core.dll`.
 3. Write the documentation within the files generated in (2).
 4. Manually merge those changes with the corresponding files from
    [Xamarin.Forms-api-docs/docs][1].

This workflow is complicated, and scary -- particularly step (4).

There is an alternative workflow: Instead of treating
xamarin/Xamarin.Forms-api-docs as the "source of truth" for API
documentation, we can instead use "conventional"
[C# documentation comments][2] to be the single source of truth.
C# documentation comments can be "inline" -- as is already done for
several files within this repo *already* -- or they can be stored in
separate XML files, via the [`<include/>` tag][3].
See [dotnet/java-interop@9d6e160c][4] for an example.

When C# documentation comments are used, the C# compiler will emit an
e.g. `Xamarin.Forms.Core.xml` file as part of the build process.

`Xamarin.Forms.Core.xml` can be fed to `mdoc update --import PATH`,
which will cause `mdoc update` to import the documentation present
within `Xamarin.Forms.Core.xml` and merge the contents into the
generated **mdoc**(5) XML files.

Note: `mdoc update` *always* prefers imported documentation over
whatever documentation already exists.

For demonstration purposes, I've:

 1. Updated `Xamarin.Forms.Core.csproj` to generate the
    `Xamarin.Forms.Core.xml` documentation file.

 2. Updated `Xamarin.Forms.Core.csproj` to *ignore* the
    CS1573 and CS1591 warnings, so that it still builds;
    it will take some time to import all existing docs into the
    xamarin/Xamarin.Forms repo, if this is desirable.

 3. Merged the API docs for
    `Xamarin.Forms.Internals.ActionSheetArguments` from
    xamarin/Xamarin.Forms-api-docs into `ActionSheetArguments.cs`

 4. Updated the `make Xamarin.Forms.Core.docs` target to use
    `mdoc update --import Xamarin.Forms.Core.xml`.

This allows the generated `docs/Xamarin.Forms.Core` directory to
contain the original C# documentation comments-based documentation.

[0]: http://docs.go-mono.com/?link=man%3amdoc(5)
[1]: https://github.com/xamarin/Xamarin.Forms-api-docs/tree/c403091dcf4f16e441f49f09b0c73315390148eb/docs
[2]: https://docs.microsoft.com/dotnet/csharp/language-reference/language-specification/documentation-comments
[3]: https://docs.microsoft.com/dotnet/csharp/language-reference/language-specification/documentation-comments#include
[4]: dotnet/java-interop@9d6e160
@hartez
Copy link
Contributor

hartez commented Oct 23, 2020

We were actually talking about this the other day; the roadblock we ran into was the public docs editing process.

Right now the source of truth for the online docs is the api-docs repo. Like most MS docs now, anyone can post issues and PR edits directly from the website; the issues and PRs are directed at the api-docs repo.

So, we would either need some sort of synchronization process to pull edits to the api-docs repo back into the /// comments in the source, OR we would need a way for the public edit PRs to be directed appropriately to the Forms repo.

Alternatively, we would need to disable the public editing process.

@samhouts
Copy link
Member

We just turned off the feedback control on the website to make this a bit simpler. But it might be a good next step to make the repo readonly if we go this route.

@hartez
Copy link
Contributor

hartez commented Oct 23, 2020

We just turned off the feedback control on the website to make this a bit simpler. But it might be a good next step to make the repo readonly if we go this route.

Which feedback control? I can still see a Feedback section on the docs pages.

@Redth
Copy link
Member

Redth commented Oct 24, 2020

I think that change may not be merged to live yet. Will be looking at this Monday.

We should get some docs people to weigh in on making the c# source the single source of truth. I do agree that the current workflow is really cumbersome and I actually think we might see more community help with improving docs especially around including them for new feature PR’s if the process was easier. Essentials has this same issue.

@jfversluis
Copy link
Member

We should get some docs people to weigh in on making the c# source the single source of truth.

@davidbritch ? Also because we were talking about this/setting this up for the Toolkit?

@mattleibow
Copy link
Contributor

We will need to check with @joelmartinez if this works nicely with the frameworks system.

We will also want to use that so we can track differences in the API on the docs site. I know I was looking at some of the issues where people aren't sure what goes with which versions.

@davidbritch
Copy link

A couple of things spring to mind from a docs perspective.

Localization

There are 14 repos for the Xamarin.Forms API docs - the original en-US source, and then 13 (private) localized repos. This is primarily to support localized intellisense. From the description I've read above, it doesn't sound like this will impact on the localization process (which is fired by check-ins to https://github.com/xamarin/Xamarin.Forms-api-docs). I'm mentioning this just in case anyone can think of a reason why it could impact on loc.

Air gapped content

There's a requirement for all content on docs.microsoft.com to work in air-gapped environments. The Xamarin conceptual docs have already been made air-gap compliant, and the API docs will be made air gap compliant soon(ish). My point here is that we want to avoid a process where Xamarin.Forms.Core.xml has some non-air gapped content, which keeps over writing an air-gapped version in the Xamarin.Forms-api-docs repo. I can think of a few ways of dealing with this, which shouldn't cause you any work.

Make the API docs repo read-only

The API docs repo must accept PRs to ensure the content is air-gapped and remains air-gapped. The solution here could be to make the repo private. I'm not sure if there's any knock-on impact if the bulk of the API docs content is made closed source in the short term (I get that the ideal is that the existing content is converted to /// and added to the source code, but I'm guessing it could be a while before that happens).

@hartez
Copy link
Contributor

hartez commented Oct 26, 2020

I'm mentioning this just in case anyone can think of a reason why it could impact on loc.

It would require a couple of minor changes in the script CI uses to add the localized docs to the NuGet packages (for Intellisense).

@joelmartinez
Copy link
Member

A few notes:

  • Switching to an "import" workflow is definitely feasible, where the source of truth is ///
    • though has @jonpryor mentioned, that means /// will always be the source of truth, and documentation contributions from the community must come through edits to the ///.
  • If you make the api docs repo readonly, and switch the workflow so that updates to the docs are simply:
      1. run mdoc to update stubs 2. include roslyn generated xml docs for import
    • That can still leave the loc workflow untouched. the xamarin.forms-api-docs repo then simply becomes the build artifact from which docs are published and loc changes are sourced.
  • To @mattleibow's point, switching to a moniker/framework-based system wouldn't necessarily work with the workflow as-is, but it could certainly work with an import//// model
    • to move to that would require that all tracked binaries (ie. previous versions) be maintained somewhere.
    • Traditionally, we've done this by having a "binaries" repository where we publish new versions to ... but that's only an implementation detail as long as previous versions are available.

All in all, there's nothing really stopping you from optimizing your docs process ... you just have to make some strong decisions about "source of truth", and whether you want to start tracking previous versions or not; the rest is just CI updates ;)

@davidbritch
Copy link

I spoke to the guy who ensures that docs content is air-gap compatible. He said you'll have to ensure that /// comments are air-gap compatible yourselves. It's no biggy though. I can discuss this further in Teams.

Suppose you want to have C# `///` docs as the "source of truth", but
you don't want to work quite so hard to *import* those docs into
C# `///` format.

This is where the [`<include/>`][0] documentation comment comes in:
since **mdoc**(5) documentation is XML, you can *copy* the `.xml`
files into this repo, and then use an XPath expression to extract
documentation from the **mdoc**(5) documentation:

	<!-- file: On.xml -->
	<Type Name="On" FullName="Xamarin.Forms.On">
	  <Docs>
	    <summary>Class that is used within <c>OnPlatform</c> tags in XAML when specifying values on platforms.</summary>
	  </Docs>
	  <Members>
	    <Member MemberName=".ctor">
	      <Docs>
	        <summary>Creates a new <see cref="T:Xamarin.Forms.On" /> with default values.</summary>
	…

	// File: On.cs

	/// <include file="On.xml" path="/Type/Docs" />
	public partial class On {
	    /// <include file="On.xml" path="/Type/Members/Member[@MemberName='.ctor']/Docs" />
	    public On()
	    {
	    }
	}

This results in a somewhat "conceptually weird" workflow:

 1. Documentation is in `On.xml`, in **mdoc**(5) format.

 2. `msbuild Xamarin.Forms.Core.csproj` generates a
    `Xamarin.Forms.Core.xml` file which contains docs from `On.xml`

 3. `make Xamarin.Forms.Core.Docs` /
    `mdoc update --import Xamarin.Forms.Core.xml …` generates *new*
    **mdoc**(5) output XML for integration w/ Xamarin.Forms-api-docs,
    based on the contents of `Xamarin.Forms.Core.xml`.

While this feels "weird" -- we start with **mdoc**(5) XML, and wind up
with **mdoc**(5) XML! -- it's useful as *transitionary* infrastructure,
making it easier to begin using C# `///` documentation comments.

Additionally, using C# `///` comments as an "intermediary" also means
that (1) doesn't *need* to be *strictly* **mdoc**(5) format.
One issue with **mdoc**(5) XML files is that method overloads are
difficult to query via XPath.  For example, consider
`AbsoluteLayout.OnChildRemoved`:

	<Type Name="AbsoluteLayout" FullName="Xamarin.Forms.AbsoluteLayout">
	  <Members>
	    <Member MemberName="OnChildRemoved">
	      <MemberSignature Language="C#" Value="protected override void OnChildRemoved (Xamarin.Forms.Element child);" />
	      <Docs>…</Docs>
	    </Member>
	    <Member MemberName="OnChildRemoved">
	      <MemberSignature Language="C#" Value="protected override void OnChildRemoved (Xamarin.Forms.Element child, int oldLogicalIndex);" />
	      <Docs>…</Docs>

They both share the same `//Member/@MemberName` value, and differ in
`//Member/MemberSignature/@Value` values and `//Member/Parameters`
values.  While they *can* be distinguished in XPath, it would be much
easier for C# `///` migration purposes to *change* the
`//Member/@MemberName` value so that they're unique, making for an
easier XPath query:

	<Type Name="AbsoluteLayout" FullName="Xamarin.Forms.AbsoluteLayout">
	  <Members>
	    <Member MemberName="OnChildRemoved">
	      <MemberSignature Language="C#" Value="protected override void OnChildRemoved (Xamarin.Forms.Element child);" />
	    </Member>
	    <Member MemberName="OnChildRemoved2">
	      <MemberSignature Language="C#" Value="protected override void OnChildRemoved (Xamarin.Forms.Element child, int oldLogicalIndex);" />

Or, the XML can be radically simplified, if desired:

	<Members>
	  <OnChildRemoved>
	    <!-- contents of previous Docs/ element -->
	  </OnChildRemoved>
	  <OnChildRemoved2>
	    <!-- contents of previous Docs/ element -->
	  </OnChildRemoved2>

The point is, step (2) provides a degree of flexibility around "where"
documentation is located, flexibility that isn't present when using
only **mdoc**(5) documentation.

[0]: https://docs.microsoft.com/dotnet/csharp/language-reference/language-specification/documentation-comments#include
@samhouts samhouts added the Core label Oct 28, 2020
@davidbritch
Copy link

The .NET Core team is doing something similar: dotnet/runtime#44969

In particular, they mention a porting tool they've written for porting API docs back to ///.

@gewarren
Copy link

@carlossanlop FYI.

@jfversluis
Copy link
Member

Not going to happen for Forms anymore I guess and for .NET MAUI I'm assuming this will follow the ".NET way"

@jfversluis jfversluis closed this Oct 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants