Page MenuHomePhabricator

Add includes/filebackend/README as generated documentation page
Closed, ResolvedPublic

Description

The generated MediaWiki PHP documentation has an Introduction section. All the sections under this in the left-hand nav (Introduction > Introduction, Setup, File Operations...) come from includes/filebackend/README. The intent in the README markup is they should appear in a separate File Backend section:

/*!
\ingroup FileBackend
\page file_backend_design File backend design

The same material appears under Modules > File backend in the left nav if you scroll down to File backend design, and there's a link to it when you click More... in the introduction to Modules > File backend. So just discourage the material from appearing in the top-level Introduction as well.

Event Timeline

Spage raised the priority of this task from to Medium.
Spage updated the task description. (Show Details)
Spage subscribed.

Change 222872 had a related patch set uploaded (by Cgardner):
filebackend Documentation Fix (T87796)

https://gerrit.wikimedia.org/r/222872

https://gerrit.wikimedia.org/r/#/c/222872

Two Doxygen commands were conflicting in the README file: \page and \ingroup. I removed \page and inserted the text as a detail description for the FileBackend module, in FileBackend.php's \defgroup.

I believe the text in question makes more sense as a detail description of the module, but the original README file is still there, conditionally excluded from Doxygen, in case someone wants to keep the extra text as a separate file.

Spage set Security to None.

Thanks for looking into this!

https://gerrit.wikimedia.org/r/#/c/222872

Two Doxygen commands were conflicting in the README file: \page and \ingroup. I removed \page

That change alone makes the text disappear from the Introduction section (good), but it also gets rid of it from the Modules >File backend section (bad).

and inserted the text as a detail description for the FileBackend module, in FileBackend.php's \defgroup.

I think that's overburdening the PHP file. It's fine to have a separate README, we have several in our codebase. We want a lightweight way to

  1. reference the README it in the source code of the relevant file
  2. have a link to the converted README doc in the generated Doxygen for the group
  3. have the README heading(s) appear in the Doxygen navigation bar on the left within that group
  4. not have the README heading(s) appear in the Doxygen navigation bar within Introduction.

We've got 2 and 3, I don't know how to achieve 1 and 4, even after reading the doxygen doc.

Doxygen 1.8 supports markdown files. I don't know if these three things are easier if we change the file to README.md.

Krinkle renamed this task from includes/filebackend/README generates duplicate documentation under "Introduction" to Add includes/filebackend/README as generated documentation page.Sep 18 2019, 4:54 PM
Krinkle updated the task description. (Show Details)
Krinkle updated the task description. (Show Details)

We want a lightweight way to

  1. reference the README it in the source code of the relevant file
  2. have a link to the converted README doc in the generated Doxygen for the group
  3. have the README heading(s) appear in the Doxygen navigation bar on the left within that group
  4. not have the README heading(s) appear in the Doxygen navigation bar within Introduction.

We've got 2 and 3, I don't know how to achieve 1 and 4, even after reading the doxygen doc.

I'm curious where 2 and 3 are visible. Right now, I'm not seeing a link to the README in group__FileBackend.html or a "File backend design" link anywhere in the sidebar. There is some longer form content that appears on classFileBackend.html, but it comes from FileBackend.php.

Change 539439 had a related patch set uploaded (by Alex Paskulin; owner: Alex Paskulin):
[mediawiki/core@master] docs: Convert class-level documentation files to Markdown

https://gerrit.wikimedia.org/r/539439

With Markdown support enabled in Doxygen, we have the option to generate this page by converting it to Markdown. I've opened https://gerrit.wikimedia.org/r/c/mediawiki/core/+/539439 as an example of what this would look like. To summarize:

  • filebackend/readme would need to be renamed to avoid being excluded by the */README.md filter. As a result, the doc would no longer appear as a directory landing page in tools like GitHub.
  • The doc could only appear in the Doxygen top-level navigation, not under the class directory as would be preferable.
  • We could link to the doc from within the commented class description, referring readers of the class page to the Markdown doc.

Including similar files for FileRepo and JobQueue, the resulting navigation looks like this:

navigation-screenshot.png (602×396 px, 51 KB)

I fiddled around a bit and found something that might work. First, the outcome:

Screenshot 2019-10-16 at 01.40.46.png (1×1 px, 345 KB)

Note how the Arch section is in the sidebar within the relevant module sub-menu, not in the root list. This is the outcome we want. Yay!

I got there by adding @ingroup FileRepo to the top of architecture.md.

The downside is that the formatting of the actual content gets funky. The first paragraph and bullet list of the architecture page renders fine, but then subsequent paragraphs don't render right. I'm not sure why that is. And, while the menu is great, the result of clicking it isn't a dedicated page. Instead, it's a "very large footnote" at the bottom of the index page for the given module group. It's essentially acting as if this Markdown file was part of the @defgroup block.

Nice! I was able to get the paragraphs to render correctly by putting @ingroup FileRepo above the title, but, as a result, Doxygen nests the file under the filename in the sidebar:

Screen Shot 2019-10-21 at 11.08.39 AM.png (1×2 px, 475 KB)

Although not ideal, I think this behavior is ok, at least for a first step.

Nice. Yeah, that seems fine. Could you upload the updated patch to Gerrit?

Yep! I'll do that later today.

Change 539439 had a related patch set uploaded (by Krinkle; owner: Alex Paskulin):
[mediawiki/core@master] docs: Convert class-level documentation files to Markdown

https://gerrit.wikimedia.org/r/539439

Change 539439 merged by jenkins-bot:
[mediawiki/core@master] docs: Convert class-level documentation files to Markdown

https://gerrit.wikimedia.org/r/539439

It looks like https://gerrit.wikimedia.org/r/539439 did not completely solve this issue. The new pages now appear nested under the code of conduct page. I'll experiment with how to resolve this, but I'm not sure how this is happening. Additionally, it looks like we'll need another exclude pattern to account for https://doc.wikimedia.org/mediawiki-core/master/php/md_maintenance_benchmarks_README.html

Change 548313 had a related patch set uploaded (by Alex Paskulin; owner: Alex Paskulin):
[mediawiki/core@master] docs: Exclude pattern for maintenance READMEs

https://gerrit.wikimedia.org/r/548313

I've opened a patch for the new exclude pattern, but I wasn't able to find a way to get the class-level Markdown files to not nest under the code of conduct while still having the Markdown formatting render correctly. We'll need to decide whether to revert https://gerrit.wikimedia.org/r/539439 if we can't find a solution for this.

Change 548313 merged by jenkins-bot:
[mediawiki/core@master] docs: Exclude pattern for maintenance READMEs

https://gerrit.wikimedia.org/r/548313

It being categorised as subpage of "Code of Conduct" is quite mysterious indeed. I am unable to find a link between these pages that would cause Doxygen to associate them.

What happens if we exclude that file from the index (given it isn't needed in the Doxygen output). Do they move to another mysterious place?

Change 549926 had a related patch set uploaded (by Alex Paskulin; owner: Alex Paskulin):
[mediawiki/core@master] (WIP) docs: Adjust ingroup Markdown files

https://gerrit.wikimedia.org/r/549926

Interesting results: When I excluded the Code of Conduct page from the Doxygen output, the new pages appeared nested under the Introduction page, which is the outcome I was expecting originally. However, when I added another Markdown file (converting one of the /docs .txt files, for example), the new page appeared nested under that Markdown page. I experimented with creating a dedicated landing page, but it seems that Doxygen randomly selects a Markdown page under which to nest the pages tagged with @ingroup. Since we eventually plan to convert the /docs .txt files to Markdown, I don't think excluding the Code of Conduct is a viable solution here.

One idea that could work is to remove the ref links from the page titles. This results in the outcome described above in which the navigation renders correctly, but paragraph breaks within the pages aren't respected. Although a bit hacky, I was able to create paragraph breaks by adding <br> tags to the ends of paragraphs. I've opened a patch to illustrate this (inelegant) solution and included a screenshot of the result:

Screen Shot 2019-11-08 at 12.22.52 PM.png (1×2 px, 461 KB)

The only other option I see is to revert the conversion to Markdown and try to render the text through another file format, although I'm not sure what this would look like.

Hm.. yeah, so for now I'm thinking maybe it's not so bad if we keep them as normal top-level pages, with "JobQueue Architecture" being there alongside "Introduction", manually linked from a related group or class description. Basically what we have now, but without the @ingroup trick.

That seems to produce no surprising side-effects and seems easy to reason about, even if it's not (yet) the hierarchy we originally wanted. Thoughts?

Sounds good to me! I'll open a patch.

Change 549926 abandoned by Alex Paskulin:
(WIP) docs: Adjust ingroup Markdown files

Reason:
See conversation in T87796

https://gerrit.wikimedia.org/r/549926

Change 550575 had a related patch set uploaded (by Alex Paskulin; owner: Alex Paskulin):
[mediawiki/core@master] docs: Remove ingroup tag from Markdown files

https://gerrit.wikimedia.org/r/550575

Change 550575 merged by jenkins-bot:
[mediawiki/core@master] docs: Remove ingroup tag from Markdown files

https://gerrit.wikimedia.org/r/550575

eprodromou subscribed.

I'm untagging Green team; I can't confirm this either way. @apaskulin can you close this if it's done?

Change 222872 abandoned by Aklapper:

[mediawiki/core@master] filebackend Documentation Fix (T87796)

Reason:

Superseded by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/550575/

https://gerrit.wikimedia.org/r/222872