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

Add doxygen target using cmake-generated doxyfile #4700

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

PhilipFackler
Copy link
Contributor

Proposed changes

This is a pretty small change to add a generated doxygen target (so you can make qmcpack_doxygen and it will put the results in the build tree). I left the old doxygen work untouched and simply added a CMakeLists.txt file to the doxygen directory. This could be further customized of course, but it provides an easy way for developers to look at an up-to-date doxygen site locally.

What type(s) of changes does this code introduce?

  • Documentation changes

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

Ubuntu 22.04

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'
  • No. Code added or changed in the PR has been clang-formatted
  • No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
  • No. Documentation has been added (if appropriate)

doxygen/CMakeLists.txt Outdated Show resolved Hide resolved
@prckent
Copy link
Contributor

prckent commented Aug 16, 2023

Agree this is useful. We will merge after the release to avoid modifying CMakeLists.txt just ahead of a release.

I assume you have found https://docs.qmcpack.org/doxygen/doxy/ , which is run nightly.

A potentially slicker and longer term solution would be to have the doxygen pages included in our readthedocs builds of the manual, but this is much more work and may need some bridging software.

@PhilipFackler
Copy link
Contributor Author

@prckent , part of the reason I wanted this is that the website you referenced (https://docs.qmcpack.org/doxygen/doxy/) seems drastically out of date. And yes, an automated build included with the rtd would be amazing.

@prckent
Copy link
Contributor

prckent commented Aug 17, 2023

Can you give an example of what is missing from the current doxygen created files so that I can check the regeneration? The website has a current date, so it is not that the whole regeneration is failing.

@quantumsteve
Copy link
Contributor

Can you give an example of what is missing from the current doxygen created files so that I can check the regeneration? The website has a current date, so it is not that the whole regeneration is failing.

The inheritance diagram for SPOSet contains derived classes such as SPOSetProxy that we couldn't find in the QMCPACK repository.

https://docs.qmcpack.org/doxygen/doxy/d4/d45/a04456.html

@prckent
Copy link
Contributor

prckent commented Aug 17, 2023

Check now. git repo updating was broken.

@quantumsteve
Copy link
Contributor

Check now. git repo updating was broken.

SPOSet looks Okay.

set(DOXYGEN_INTERACTIVE_SVG YES)
set(DOXYGEN_DISTRIBUTE_GROUP_DOC YES)

doxygen_add_docs(qmcpack_doxygen ${PROJECT_SOURCE_DIR}/src
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer

    doxygen_add_docs(qmcpack_doxygen ${PROJECT_SOURCE_DIR}/src)
else()
    message(STATUS "The qmcpack_doxygen target for the QMCPACK developer/API documentation requires (1) doxygen and (2) dot from graphviz.")
endif()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a message to similar effect

find_package(Doxygen)
if(DOXYGEN_FOUND)
set(DOXYGEN_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}")
set(DOXYGEN_USE_MDFILE_AS_MAINPAGE ${PROJECT_SOURCE_DIR}/README.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to remove this, since it gives a very strange first page.

Can you modify the config here so that it gives an accessible start page like https://docs.qmcpack.org/doxygen/doxy/index.html ?
The one produced by this PR has a long function(?) list before the classes and files browser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prckent I believe the issue was with the navigation tree on the left. I like using the README since it gives links to all the relevant things on the web. I addressed the navigation tree issues (and in my opinion it's more useful than the tree in the current published version). Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that if you don't want to see the main page headings in the navigation, they can be removed with:
set(DOXYGEN_TOC_INCLUDE_HEADINGS 0)

@PDoakORNL
Copy link
Contributor

I still have interest in this. Will certainly make cleaning up doxygen output locally more convenient. It really should be part of dev workflow to see if our doxygen output for new work looks good or not.

@prckent
Copy link
Contributor

prckent commented Dec 6, 2023

@PhilipFackler Will you be able to come back to this? This looked to be mergable after fixing the first page (DOXYGEN_USE_MDFILE_AS_MAINPAGE) to be more sensible somehow. Can be a very simple .md of your own creation.

@prckent
Copy link
Contributor

prckent commented Dec 12, 2023

(I expect we'll merge this after it has been run independently, after the QMCPACK workshop this week)

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.

None yet

5 participants