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

Documenation build script - add check for missing docs files #250

Conversation

GenevieveBuckley
Copy link
Collaborator

I've noticed that it's very easy to add a new documentation page and then forget to include it in the micro_sam/__init__.py module docstring. This means pdoc can't find it, and it will never appear on the website.

This PR:

  1. Adds a check to the build_docs.py script, that looks for any missing markdown or RST files that exist in the "doc" folder but are not included in the micro_sam/__init__.py docstring. It raises a RuntimeError if any missing files are found, with instructions on how to fix it.
  2. Edits the .github/workflows/build_docs.yaml script so that it uses the same build_docs.py script to generate the documentation. Advantages: (i) we can use the same check for missing files automatically, and (ii) it's simpler to use the same process everywhere
  3. Adds the currently missing doc files band.md and development.md to the docstring in micro_sam/__init__.py. Now, pdoc will be able to find this content and include it on the website
  4. Changed the heading levels the markdown file band.md. By making the subsections have level 2 markdown headings, instead of just bold text, the different sections will appear in the left hand sidebar menu of the website, and people can click on them to go directly to that section.

@GenevieveBuckley
Copy link
Collaborator Author

Also - I've opened this PR against the master branch, because I think we need it everywhere. Is that the right way to go?

I notice that the band.md documentation file does not exist on the dev branch, only in master, and that made me a little confused about where my pull request needed to go. It's possible dev and master have drifted a little bit out of sync, might need to check that.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

❗ No coverage uploaded for pull request base (master@ebc53e0). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #250   +/-   ##
=========================================
  Coverage          ?   38.40%           
=========================================
  Files             ?       30           
  Lines             ?     3999           
  Branches          ?        0           
=========================================
  Hits              ?     1536           
  Misses            ?     2463           
  Partials          ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@constantinpape
Copy link
Contributor

Also - I've opened this PR against the master branch, because I think we need it everywhere. Is that the right way to go?

Yes, that's ok.

I notice that the band.md documentation file does not exist on the dev branch, only in master, and that made me a little confused about where my pull request needed to go. It's possible dev and master have drifted a little bit out of sync, might need to check that.

Yes, they got a little out of sync. I pushed BAND to the master branch to make it easier to access by a collaborator who is using it. (But it should be included in the documentation, since it is a generally useful feature).

In general: we should merge changes into dev if they change existing functionality, but it's ok to merge on master if this is not the case (as here).

@constantinpape constantinpape merged commit 12d2518 into computational-cell-analytics:master Oct 29, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants