mono / api-doc-tools

.NET Reference API Toolchain
MIT License
68 stars 48 forks source link

Large file warning for index.xml #423

Closed mairaw closed 5 years ago

mairaw commented 5 years ago

As we continue to onboard more APIs, the size of the index.md file continues to grow and now reached GitHub’s maximum recommended size:

remote: Resolving deltas: 100% (2513/2513), completed with 1273 local objects. remote: warning: GH001: Large files detected. You may want to try Git Large File Storage - https://git-lfs.github.com. remote: warning: See http://git.io/iEPt8g for more information. remote: warning: File xml/index.xml is 50.20 MB; this is larger than GitHub's recommended maximum file size of 50.00 MB To https://github.com/mairaw/dotnet-api-docs.git f29bb20fa9..0d43acfb00 master -> master

index.xml is currently used to show the extension methods for a given class.

Is there something we can do to mitigate this?

TianqiZhang commented 5 years ago

take one extension method for example:

    <ExtensionMethod>
      <Targets>
        <Target Type="T:Microsoft.Win32.RegistryKey" />
      </Targets>
      <Member MemberName="GetAccessControl">
        <MemberSignature Language="C#" Value="public static System.Security.AccessControl.RegistrySecurity GetAccessControl (this Microsoft.Win32.RegistryKey key);" />
        <MemberSignature Language="ILAsm" Value=".method public static hidebysig class System.Security.AccessControl.RegistrySecurity GetAccessControl(class Microsoft.Win32.RegistryKey key) cil managed" />
        <MemberSignature Language="DocId" Value="M:Microsoft.Win32.RegistryAclExtensions.GetAccessControl(Microsoft.Win32.RegistryKey)" />
        <MemberSignature Language="VB.NET" Value="&lt;Extension()&gt;&#xA;Public Function GetAccessControl (key As RegistryKey) As RegistrySecurity" />
        <MemberSignature Language="C++ CLI" Value="public:&#xA;[System::Runtime::CompilerServices::Extension]&#xA; static System::Security::AccessControl::RegistrySecurity ^ GetAccessControl(Microsoft::Win32::RegistryKey ^ key);" />
        <MemberSignature Language="F#" Value="static member GetAccessControl : Microsoft.Win32.RegistryKey -&gt; System.Security.AccessControl.RegistrySecurity" Usage="Microsoft.Win32.RegistryAclExtensions.GetAccessControl key" />
        <MemberType>ExtensionMethod</MemberType>
        <ReturnValue>
          <ReturnType>System.Security.AccessControl.RegistrySecurity</ReturnType>
        </ReturnValue>
        <Parameters>
          <Parameter Name="key" Type="Microsoft.Win32.RegistryKey" RefType="this" />
        </Parameters>
        <Docs>
          <param name="key">To be added.</param>
          <summary>To be added.</summary>
        </Docs>
        <Link Type="Microsoft.Win32.RegistryAclExtensions" Member="M:Microsoft.Win32.RegistryAclExtensions.GetAccessControl(Microsoft.Win32.RegistryKey)" />
      </Member>
    </ExtensionMethod>

You can see it's just a normal member with a target. So how about put it to the type xml where it was defined? @joelmartinez

joelmartinez commented 5 years ago

@TianqiZhang All of these extension methods are in the type XMLs where they were defined :) For this particular method, you can find it here:
https://github.com/dotnet/dotnet-api-docs/blob/master/xml/Microsoft.Win32/RegistryAclExtensions.xml#L29

The "extra" information in the ExtensionMethod node is the Target, which is just the RefType="this" Parameter's Type … and the Link is just where this extension method is defined.

supernova-eng commented 5 years ago

I would suggest we re-visit the idea of including this information in {framework}.xml. The current file size for the .NET Framework 4.8 index (there is more than one index, right? 😄) is 15.8MB - it's quite a bit away from the 50MB limit, so I think we can just integrate the information there and avoid creating additional configuration/index files.

mairaw commented 5 years ago

There's a general index.xml directly under the xml folder: https://github.com/dotnet/dotnet-api-docs/blob/master/xml/index.xml

That's the one I was referring to that has 51.6Mb now.

TianqiZhang commented 5 years ago

Sorry @joelmartinez I must've looked in the wrong file 😄

So based on the information we have right now, when ECMA2Yaml sees a static method in a static class and that method contains a parameter with RefType="this":

Is this logic correct? If so then ECMA2Yaml does not need index.xml to identify extension methods, I can make that change in the future.

joelmartinez commented 5 years ago

@TianqiZhang that's 100% correct :)

@dendeli-msft, @mairaw once Ecma2Yaml makes this change, in the short term, we'll be able to simply delete index.xml … and in the long term I'll close this gh issue by making it so that mdoc update in frameworks mode won't generate an index.xml :)

TianqiZhang commented 5 years ago

@dendeli-msft do we need a separated feature or epic to track this?

wh-alice commented 5 years ago

I just create a dev task for the tracking. @mairaw , could you please help to share the priority from your perspective?

joelmartinez commented 5 years ago

Just created additional internal items for mdoc-specific work related to this:

mairaw commented 5 years ago

@wh-alice as I mentioned on the meeting yesterday, I think this one is lower priority than others and perhaps could even be moved to 5.7.5. My only fear is that the file size is quickly growing and might reach the max size in a few weeks.

joelmartinez commented 5 years ago

@mimisasouvanh ... I know @mairaw mentioned above that it's a lower priority, but you should follow up and see where the filesize is today, and how close it is to reaching the limit.

mairaw commented 5 years ago

Its currently at 65.6 MB.

mairaw commented 5 years ago

And you can check the size at any time by going to https://github.com/dotnet/dotnet-api-docs/blob/master/xml/index.xml.

image

TianqiZhang commented 5 years ago

ECMA2Yaml has removed its dependency on index.xml now. So feel free to retire it in .NET CI, nothing will be impacted.