Files icon indicating copy to clipboard operation
Files copied to clipboard

Code Quality: Document entire codebase

Open Lamparter opened this issue 1 year ago • 2 comments

Description

Since the (unfortunate) closure of #15652, something that I had been planning to do since ~2021, I've been re-motivated to start a super-documentation of the entire codebase.

Concerned code

Every single file and folder in the codebase.

Gains

This would make Files a better place to learn technologies such as C#/WinAppSdk (which both @0x5bfa and I can tell it definitely is good for!) especially considering that Files is practically the biggest, most popular WinUI3 app of all time. (well, 3rd party at least). This would also:

[organise the repo] for newbies to be able to participate easily.

Requirements

There are numerous possible solutions for this problem:

  • Document functions with C#/Spec
/// <summary/>
/// Write a summary of the function here...
/// </summary>
public string MyValue => "Hello World!";

This would allow for using a tool to generate it into documentation (periodically) that could be hosted on the website or in the codebase directly.

  • README.md (markdown) style documentation

For example, as introduced in #16363 and #15652. This implements directly with the GitHub UI and displays the documentation in the folder tree.

  • Simply add lots of comments to code

Comments

This is a rather large proposal but I don't see any reason against it, the worry of outdated docs is a non-issue.

Lamparter avatar Oct 18 '24 17:10 Lamparter

which both @0x5bfa and I can tell it definitely is good for!

I said good codebase is good but making such markdowns is a bad idea. Those who know of MVVM they understand what the readmes say without them. And I strongly recommended to get approval before working on this to avoid extra work. Please read the guideline.

the worry of outdated docs is a non-issue.

This is the very issue.

0x5bfa avatar Oct 18 '24 18:10 0x5bfa

This is the very issue.

I said it's a non-issue because it is an open source codebase and it is not up to maintainers such as Yair to do all the work themselves.

Lamparter avatar Oct 19 '24 15:10 Lamparter

Can you clarify what you mean by documenting folders?

yaira2 avatar Oct 27 '24 01:10 yaira2

Docs tend to get outdated, but an active community can remedy that 🙂

yaira2 avatar Oct 27 '24 01:10 yaira2

Docs tend to get outdated, but an active community can remedy that 🙂

Yes that's what I said to @0x5bfa 😄

By documenting folders, I meant adding a README file that explains what goes on in every folder. However, Files follows a fairly self-explanatory MVVM structure so this solution might not be so useful as simply adding comments to the code and generating documentation from that.

Lamparter avatar Oct 27 '24 08:10 Lamparter

Having to go to each folder is inconvenient I guess. What about this? https://github.com/microsoft/CsWinRT/blob/master/docs/structure.md

0x5bfa avatar Oct 27 '24 13:10 0x5bfa

Hmm I don't think it's as good as just using specification markers in code

Lamparter avatar Oct 27 '24 13:10 Lamparter

We had a long discussion about this and resolved that something like this would benefit Files best, perhaps in this case on the website of course. This would pair nicely with an effort to use documentation comments in the code (i.e. /// markers) across the codebase as this would be more efficient and would allow documentation to be generated perhaps via a cron worker.

Lamparter avatar Oct 28 '24 15:10 Lamparter

I would suggest to create ones in docs folder in the Files repo. That said, docs of specs and features would go to the Website.

0x5bfa avatar Oct 28 '24 18:10 0x5bfa

I think the docs should be on the website as it's where the documentation is already.

The spec markers will already be in the code and most documentation generators are aimed at Node websites rather than markdown files in a directory.

Lamparter avatar Oct 28 '24 18:10 Lamparter

Why contributors have to open two PRs on different places when they change codebase like quality changes? Specs and features are decided by us and we wouldn't mind that but we shouldn't force contributors to do so.

0x5bfa avatar Oct 28 '24 18:10 0x5bfa

Why contributors have to open two PRs on different places? Specs and features are decided by us and we wouldn't mind that but we shouldn't force contributors to do so.

You misunderstood. Contributors/members of the Files Organisation write docs using /// code markers. A cron job on the website generates the documentation and feeds it into the site automatically.

Lamparter avatar Oct 28 '24 18:10 Lamparter

Aren't you talking about readme-s in every folder? XML docs would be going to be generated into markdown by a C# generator that exists already. https://github.com/neuecc/MarkdownGenerator

0x5bfa avatar Oct 28 '24 18:10 0x5bfa

Aren't you talking about readme-s in every folder? XML docs would be going to be generated into markdown by a C# generator that exists already. https://github.com/neuecc/MarkdownGenerator

No, I was talking about regular generated documentation. There would be a cron worker that automatically generates documentation and uploads it to the website repo via a PR. The developer wouldn't expect the docs to be in two places.

Lamparter avatar Oct 28 '24 18:10 Lamparter

In that case, I agree. As for that I menationed above, it'd be better in the docs folder in the Files repo.

0x5bfa avatar Oct 28 '24 18:10 0x5bfa

A cron job on the website generates the documentation and feeds it into the site automatically.

I've wanted to do something like this for a while. I think this is the best solution and will make maintaining the docs very easy.

yaira2 avatar Oct 28 '24 18:10 yaira2

But unless we publish some kind of SDK, it's useless. What I wish I could have when I was new is structure documentation and codebase problems we have (like notes you have to keep in mind when trying to understand the codebase), what's this or that. So this PR #15652 was my hope back then.

As of now I think it's nice to have something like this:

Files Repo Structure

src

...

Files.App

...

Files.Core

...

tests

Files.App.UITests

...

0x5bfa avatar Nov 03 '24 03:11 0x5bfa