Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfc13: use function directive to make PMI API docs more readable #358

Merged
merged 4 commits into from
Dec 15, 2022

Conversation

garlick
Copy link
Member

@garlick garlick commented Dec 15, 2022

Just a bit of cleanup in RFC 13 which I had been referring to lately. Now the PMI function defintions look more like Jansson's API docs!

Problem: RFC 13's use of headings is slightly weird compared to the
sphinx style guide's recommendations.

Use the recommended H1, H2, H3, and H4 heading symbols.

See also:
https://documentation-style-guide-sphinx.readthedocs.io/en/latest/style-guide.html
Problem: API functions are shown as code blocks rather than
using the function directive, which produces a more readable
rendering.

Use the c:function:: directive where applicable.
Problem: references to previously defined functions are not linked.

Use :func: to make a function name refer to its defintion.
@grondo
Copy link
Contributor

grondo commented Dec 15, 2022

The github workflow config needs an update, might as well throw it in here?

commit c880d810f015e0ab01e0fc37b26181fcfcd41020
Author: Mark A. Grondona <mark.grondona@gmail.com>
Date:   Wed Dec 14 18:42:24 2022 -0800

    ci: update python and checkout actions

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index e351089..a2e607d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -5,7 +5,7 @@ jobs:
     runs-on: ubuntu-latest
     if: github.event_name == 'pull_request'
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
       with:
         ref: ${{ github.event.pull_request.head.sha }}
         fetch-depth: 0
@@ -16,12 +16,12 @@ jobs:
     name: make check
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
       with:
         ref: ${{ github.event.pull_request.head.sha }}
-    - uses: actions/setup-python@v1
+    - uses: actions/setup-python@v4
       with:
-        python-version: 3.6
+        python-version: '3.9'
     - name: install dependencies
       run: |
         pip3 install --upgrade -r ./requirements.txt
@@ -33,12 +33,12 @@ jobs:
     name: make linkcheck
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
       with:
         ref: ${{ github.event.pull_request.head.sha }}
-    - uses: actions/setup-python@v1
+    - uses: actions/setup-python@v4
       with:
-        python-version: 3.6
+        python-version: '3.9'
     - name: install dependencies
       run: |
         pip3 install --upgrade -r ./requirements.txt                                                                                  

Maybe try that?

@grondo
Copy link
Contributor

grondo commented Dec 15, 2022

Great! Looks nice! I noticed a strangeness in the sections listed along the sidebar, but my guess is that this was already a problem;
sc

Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

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

Nice! LGTM.

@vsoch
Copy link
Member

vsoch commented Dec 15, 2022

The nesting in the sidbar TOC is usually controlled where the menu is defined, e.g.,

.. toctree::
   :caption: Getting started
   :name: getting_started
   :hidden:
   :maxdepth: 2

So if possible, if there's nothing else at that level, you could nix it. (maxdepth -1). If there is, you'd probably need another trick: https://stackoverflow.com/questions/15035546/hide-sphinx-subsections-from-main-toctree.

Also, has there been feedback about the style of the docs? I personally find the default readthedocs template hard to move around, or kind of depressing if/when I see it that nobody changed anything. One thing I wanted to bring up for discussion is that maybe part of users being better able to find things and better enjoy / contribute to the docs overall would be to reformat into a nicer template? Here are some randoms that I've done that all use sphinx (and I even use markdown for a few for easier user contribution!)

I think we have an opportunity moving forward to really make the docs shine, and part of that, although it's not "cool work" is branding. E.g., this page https://flux-framework.org/ to any trained eye is a GitHub pages theme called Cayman https://pages-themes.github.io/cayman/ with maybe a color changed. And I'm not saying we have to go this route, but now we have two flux associated projects with these docs:

If the team wants to chat about these ideas, and if there is a desire to have a nice, unified portal for flux projects (e.g., akin to https://software.llnl.gov/radiuss/projects/ but for fewer projects and rebranded for flux) and then more unified branding of the individual project docs, I could probably help. It will make a difference, I think, when new users are trying it out and forming impressions, and/or finding/not finding stuff easily (re: the email earlier today). Let me know what you think!

@vsoch
Copy link
Member

vsoch commented Dec 15, 2022

And to express some of my criticism of the default template, even on a small screen there is like, a swimming pool sized block of unused space on the right:

Screenshot 2022-12-14 at 21-57-55 Welcome to Flux’s documentation! — Flux documentation

Oh look, someone is having a pool party! 🐩

pool-party

But even a small change of template design all of a sudden opens up the docs and uses that space for text, graphics, tutorials, and often moving the page navigation to the right and main on the left, to have both without having to choose one. And the docs are more open and easy on the eyes in not requiring more content shoved into a smaller space.

Co-authored-by: Mark Grondona <mark.grondona@gmail.com>
@garlick
Copy link
Member Author

garlick commented Dec 15, 2022

Thanks @vsoch - I made several attempts with the toctree directives and, getting nowhere, decided that it was probably a misstep to put headings under function defs that are not themselves headings, so I just dropped that commit from this PR.

Regarding the feedback on the overall docs format, what you're saying makes sense and when someone has some time I agree that we could use a makeover and some consistent branding. I'll transfer your comments into an issue in the flux-docs repo so we can track this.

@garlick
Copy link
Member Author

garlick commented Dec 15, 2022

Thanks for the approval @grondo - I'll set MWP.
Edit: and for the CI fix!

@mergify mergify bot merged commit 42ff6c9 into flux-framework:master Dec 15, 2022
@garlick garlick deleted the pretty_rfc13 branch December 16, 2022 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants