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

docs: Update Sphinx and myst-parser versions; resolve warnings #4581

Merged
merged 10 commits into from
Dec 18, 2024

Conversation

giacob500
Copy link
Contributor

@giacob500 giacob500 commented Nov 21, 2024

Summary

This PR is intended to address issue #4311 (docs: upgrading sphinx & myst-parser)

  • Updated Sphinx to version 8.0.2 and myst-parser to version 4.0.0.
  • Successfully built the documentation.
  • Encountered warnings about certain documents not being included in any toctree.

Details

While building the documentation using make docs, I received the following warnings:

checking consistency... cve-bin-tool/doc/how_to_guides/vex_generation.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/mismatch_cli.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/sboms_for_humans/cve-bin-tool-py3.10.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/sboms_for_humans/cve-bin-tool-py3.11.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/sboms_for_humans/cve-bin-tool-py3.12.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/sboms_for_humans/cve-bin-tool-py3.7.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/sboms_for_humans/cve-bin-tool-py3.8.md: WARNING: document isn't included in any toctree
cve-bin-tool/doc/sboms_for_humans/cve-bin-tool-py3.9.md: WARNING: document isn't included in any toctree

I'm unsure whether these documents should be added to the toctree or if they're intentionally excluded.

Questions

  • Should I include these documents in the toctree?
  • If so, where would be the appropriate place to include them?
  • If not, is it acceptable to suppress these warnings?

Additional Notes

  • I haven't made changes to address these warnings yet, pending your guidance.
  • Any feedback or suggestions would be greatly appreciated.

@terriko
Copy link
Contributor

terriko commented Nov 21, 2024

I think if we're bothering to generate documents, it makes more sense to include them so they're reachable, so we might as well fix all those issues. Suggestions:

  • the vex one should be included with the other how to guides
  • the mismatch one probably can also go with the how to guides for now. I'm open to other suggestions if somewhere else looks more reasonable.
  • the sbom for humans stuff should have its own toctree (and that in turn should be linked to the main toctree so people can find them if they want)

The last one is a little less important since I think people coming to look at the sboms are more likely to be browsing through git than the documentation, but who knows, maybe having that info on readthedocs will be useful to someone, and we've got a low-key goal of making sboms more accessible and usable so this seems to fit into that.

@terriko terriko changed the title Update Sphinx and myst-parser versions; resolve documentation warnings docs: Update Sphinx and myst-parser versions; resolve warnings Nov 21, 2024
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

I edited the title so it should pass the gitlint check next time the job runs (it won't update until you make changes).

It looks like spelling is complaining about 132 missing items. Since there's a lot of package and human names in there, I'd suggest we just add the whole sboms directory to the spelling exclude list, which can be found here: https://github.com/intel/cve-bin-tool/blob/main/.github/actions/spelling/excludes.txt

@giacob500
Copy link
Contributor Author

In my penultimate commit I addressed the following points:

  • the vex one should be included with the other how to guides
  • the mismatch one probably can also go with the how to guides for now
  • the sbom for humans stuff should have its own toctree (and that in turn should be linked to the main toctree so people can find them if they want)

I spent a few time trying to include hyperlinks in the sbom for humans documentation to directly open the json files but did not find a way that worked, attempts carried:

  • Including the .json files in sbom/sboms_for_humans/index.rst does not work and including them
  • Including the .json files in the sbom/sboms_for_humans/README.md file of the directory, which after building the sphinx documentation preserves the format and content of the README.md, but messes up with the internal hyperlinks. Thus the output README.html links are broken.

In my last commit I finally updated requirements.txt with the updated versions of Sphinx and myst-parser. Let me know your feedback. Thanks

@giacob500 giacob500 marked this pull request as ready for review December 8, 2024 13:01
@giacob500 giacob500 requested a review from terriko December 8, 2024 13:02
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

You've got a couple of extra files you'll need to delete: doc/=4.0.0 and doc/=8.0.2 -- looks like a couple of logs that got checked in by mistake.

terriko and others added 2 commits December 18, 2024 10:39
testing to see if I can clean up those extra files for you
removing un-needed files
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Looks like the web interface actually lets me remove the files for you, so I've gone ahead and done that. I think this should be good to go now, thank you again for working on it!

@terriko terriko enabled auto-merge (squash) December 18, 2024 18:41
@terriko terriko merged commit 7c888f1 into intel:main Dec 18, 2024
24 checks passed
@giacob500
Copy link
Contributor Author

Thanks for taking the time to do that on my behalf @terriko !

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