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

Automated documentation test for new PR #9953

Open
1 of 7 tasks
Tracked by #748 ...
ferdymercury opened this issue Feb 22, 2022 · 6 comments
Open
1 of 7 tasks
Tracked by #748 ...

Automated documentation test for new PR #9953

ferdymercury opened this issue Feb 22, 2022 · 6 comments

Comments

@ferdymercury
Copy link
Contributor

ferdymercury commented Feb 22, 2022

Explain what you would like to see improved

I've been working (with couet and others) on reducing errors in the Doxygen documentation of the code, from 10k+ to a manageable level of under 1k (and going down). However, currently, there is no safeguard to prevent new commits from introducing new errors, which is prone to happen, as not everyone is familiar with the doxygen syntax. To maintain in a sustainable way the 'clean status' of the reference guide and online documentation, it would be helpful to add an automation script that compiles the documentation only with the files modified in the Pull Request to be merged, in the same way that a bot checks now about clang-format, etc.

Because Doxygen can be configured to have only one input file, this extra check will not involve much time as compared to building the whole documentation. Then, it could be checked whether no warnings are produced by the 'touched' files.

Optional: share how it could be improved

  • The Github action or CI script should modify the -DDOCU_INPUT CMake flag to only add those touched by the Pull Request
  • The INCLUDE_PATH in the Doxyfile should be extended (for good, not just in the script) to all possible include folders, as you might touch a 'source file', but would get a bunch of warnings if Doxygen does not find the corresponding include path. (This is not the case in the full build, as the include files are also part of the INPUT, but wouldn't be the case if we just 'touch' the sources in the PR).
  • The WARN_LOGFILE (-DDOCU_LOGFILE in cmake) should be set by the CI script to a useful filename. If the file is empty after running doxygen, the Test passes.
  • Optional: HAVE_DOT, listLibs, etc. could be set to NO in case a speedup is wanted.
  • Potentially hook it also up with spell-checking checks, see https://github.com/codespell-project/codespell
  • Alternative approach suggested by albert-github to avoid missing references: use the overnight tag file from https://root.cern/reference/ with the pull request update test
  • makedoc log parser rules tuning jenkins-pipelines#10 jenkins parsing log fine-tune

Additional context

See https://root.cern/for_developers/doxygen/

@couet
Copy link
Member

couet commented Feb 23, 2022

Sounds a good idea !

@ferdymercury
Copy link
Contributor Author

Status update: With the latest commits on #9966
one can now overwrite the input via command line argument, (as well as the number of threads):

cmake -DDOCU_THREADS=N -DDOCU_INPUT="../../core/base/myModifiedFile.h" ....

so that it will be possible to only run doxygen on the 'touched files'

@ferdymercury
Copy link
Contributor Author

For having a benchmark example, right now, touching e.g. "TApplication" would run the doxygen-check in 7 seconds, thus it's well suited for a CI script.

If we prefer to run not just these two files as Input, but the whole subfolder (subproject), core/base, then it would take 1 minute and 16 seconds.

@ferdymercury
Copy link
Contributor Author

An example on how documentation generation is run within GitHub actions, so that it appears as "Check" within each PR:
https://github.com/doxygen/doxygen/blob/master/.github/workflows/build_cmake.yml
(For that, one would need first #9966)

@Axel-Naumann
Copy link
Member

It would be even better to correlate the diff with the doxygen warnings: run doxygen on the changed files and count warnings, apply the PR change, re-run doxygen and compare the warning count.

We are currently revamping our CI; this could be included in the future @olemorud

@ferdymercury
Copy link
Contributor Author

ferdymercury commented Jun 30, 2023

Hi @Axel-Naumann , I love the gcc-problem-matcher

image

It would be amazing if something equivalent could be done with the Doxygen warnings that appear now on jenkins :D

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants