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

Improve GitHub and Doxygen docs. #228

Closed
dbaarda opened this issue Aug 12, 2021 · 2 comments
Closed

Improve GitHub and Doxygen docs. #228

dbaarda opened this issue Aug 12, 2021 · 2 comments

Comments

@dbaarda
Copy link
Member

dbaarda commented Aug 12, 2021

The github actions doing the document build is spitting out the following message;

Run cmake --build '/home/runner/work/librsync/librsync/build' --target doc
  cmake --build '/home/runner/work/librsync/librsync/build' --target doc
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
[100%] Generating API documentation with Doxygen
/home/runner/work/librsync/librsync/CONTRIBUTING.md:95: warning: unable to resolve reference to 'NEWS.md' for \ref command
/home/runner/work/librsync/librsync/doc/versioning.md:17: warning: unable to resolve reference to 'NEWS.md' for \ref command
/home/runner/work/librsync/librsync/README.md:83: warning: unable to resolve reference to 'NEWS.md' for \ref command
/home/runner/work/librsync/librsync/README.md:83: warning: unable to resolve reference to 'NEWS.md' for \ref command
[100%] Built target doc

These errors don't show up if you do an in-place build, only if you specify a build directory. Interestingly it doesn't complain about any other markdown docs or links. Looking at the generated html directory I see the following files;

$ ls build/html/md_*
build/html/md__home_abo_src_librsync_librsync_NEWS.html  build/html/md__home_abo_src_librsync_librsync_TODO.html

Note that its putting the whole path to my repo into the file name, so the result is not hermetic and is leaking information about the build location. It doesn't do this for an in-place build;

$ ls html/md_*
html/md_NEWS.html  html/md_TODO.html

Also interesting is it doesn't complain about any other markdown docs, or even generate md_*.html files for them. Instead files like CONTRIBUTING.md get generated as html/page_contributing.html. It seems this is because of the {#page_contributing} on the title in these documents.

When using a newer Doxygen it doesn't complain about the references using markdown [NEWS.md](NEWS.md) style links instead of a Doxygen \ref links, but it still complains about the Doxygen \ref links. So between 1.8.17 and 1.9.1-1 Doxygen became more robust about resolving markdown references.

When viewing docs on github, all the Doxygen \ref links and {#page_install} anchors are not interpreted, making the documents harder to navigate on github. Since Doxygen understands markdown links and other markdown syntax, we should try to stick to markdown format as much as possible. However, given how Doxygen generates filenames for non-in-place builds, we probably should put {#page_<foo>} style anchors on all markdown docs.

The general structure of the docs could use some tidying. In particular the README.md does not make a great introduction/overview page. There are also other bugs related to parts of the documentation that could be improved. I have some old thoughts on documentation and document templates that would be worth considering here;

http://minkirri.apana.org.au/~abo/projects/prjdocs/

@dbaarda dbaarda changed the title Improve documentation structure for github and Doxygen. Improve documentation for github and Doxygen. Aug 20, 2021
@dbaarda
Copy link
Member Author

dbaarda commented Aug 20, 2021

Other things I've noticed;

  1. Currently .gitattributes marks *.in files as binary so that test data using this extension doesn't get modified on Windows checkouts and break tests. Unfortunately this extension is also used for some cmake template files, including doc/Doxyfile.in. Interestingly src/config.h.cmake doesn't use .in but it probably should since .cmake indicates a cmake config file, not a template. We should rename the test data to use .input instead, and rename src/config.h.cmake to src/config.h.in while we are at it.
  2. In doc/Doxyfile.in there is a setting HAVE_DOT = NO. This turns of all the nice diagrams that Doxygen can generate. Changing this to on makes the docs much more useful. There is also DOT_IMAGE_FORMAT = png, and using svg would probably be better. Also maybe interesting would be INTERACTIVE_SVG = YES to add interactive zoom/pan. Note this requires gaphviz to be installed, so we should update the GitHub action to install it.
  3. There are many files missing the basic \file <foo>.h Doxygen docstring which means they are omitted from the docs. Adding at least all the headers would make navigating the include diagrams much nicer.
  4. Some functions are documented in the *.h file, and some in the *.c file. We should put documentation in the *.h file when we can.

@dbaarda dbaarda changed the title Improve documentation for github and Doxygen. Improve GitHub and Doxygen docs. Aug 20, 2021
@dbaarda
Copy link
Member Author

dbaarda commented Aug 23, 2021

After playing around with markdown docs and github actions in #230 I think maybe replacing \ref links with markdown links may be premature. It seems the version of Doxygen on the github actions ubuntu_latest target suffers from doxygen/doxygen#7595 so markdown links don't quite work as advertised.

There are still several things here that need fixing, so I'm going to put together a PR with the bits still worth doing.

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

No branches or pull requests

1 participant