Add doxygen to document cpp code

[hisergiorojas]

Fixes #708

Summarize your change.

We made some progress but we need some feedback so we know if we're going in the right direction.
So far we have done:

• Added a new make target for Doxygen
• Converted cpp comments to follow Doxygen comment convention for one file: anyDictionary.h

[ssteinbach]

Its looking like a good start! Couple of small notes:

• I don't think we want the conf.py to have to talk to doxygen by default (keeping the sphinx python documentation and the doxygen C++ documentation somewhat separated is ok)
• I like that you added a target in the Makefile! thats helpful
• We (meaning the TSC) need to find a place to host the documentation that isn't ReadTheDocs so that we can put this documentation up in the same place. We may have already talked bout this and I'm forgetting, but it'll likely be up on GitHub pages. Regardless, we'll also want to add a GitHub action to build the documentation and do a publish whenever a commit happens (which is what happens with readthedocs at present). That could be a separate PR though. For now what I would want from this PR is a make target that produces the doxygen documentation. We can iterate on it from there!

Thanks for taking a stab at this!

[meshula]

Looks promising! It would be worth, as part of the PR, adding a note in our "building opentimelineio" docs that the doxygen docs can be generated by

cd doxygen ; doxygen config/dox_config ; cd ..

[utsab]

Thank you @ssteinbach and @meshula for your feedback. A few clarification questions to confirm that I'm understanding you correctly:

1. We do not need to integrate with readthedocs for this PR. Therefore, we should remove our changes (the references to doxygen) from conf.py.

2. The main thing you are looking for on this PR is the make target that generates the doxygen documentation (which is completed)

3. The only thing missing from this PR is @meshula's request to add a note to readthedocs on how to generate the doxygen documentation.

Are these points correct? Thank you.

[codecov-io]

Codecov Report

Merging #878 (13cc01e) into master (546b333) will decrease coverage by 0.87%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #878      +/-   ##
==========================================
- Coverage   86.63%   85.76%   -0.88%     
==========================================
  Files         183      191       +8     
  Lines       17874    18109     +235     
  Branches     1971     2048      +77     
==========================================
+ Hits        15486    15532      +46     
- Misses       1904     2055     +151     
- Partials      484      522      +38     

Flag	Coverage Δ
unittests	85.76% <ø> (-0.80%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/opentimelineio/anyDictionary.h	100.00% <ø> (ø)
src/opentimelineio/anyVector.h	94.11% <ø> (ø)
src/opentimelineio/serializableObject.h	88.88% <ø> (-1.02%)	⬇️
src/opentimelineio/serialization.cpp	81.52% <ø> (-2.62%)	⬇️
src/opentimelineio/typeRegistry.h	100.00% <ø> (ø)
src/opentimelineio/safely_typed_any.cpp	80.76% <0.00%> (-15.07%)	⬇️
...entimelineio/opentimelineio/console/otioconvert.py	76.28% <0.00%> (-14.74%)	⬇️
...ineio/opentime-bindings/opentime_timeTransform.cpp	90.00% <0.00%> (-10.00%)	⬇️
src/opentimelineio/clip.cpp	82.60% <0.00%> (-7.87%)	⬇️
src/opentimelineio/trackAlgorithm.cpp	70.00% <0.00%> (-7.78%)	⬇️
... and 80 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 546b333...13cc01e. Read the comment docs.

[meshula]

Thanks for adding build notes for me :)

[ssteinbach]

1. We do not need to integrate with readthedocs for this PR. Therefore, we should remove our changes (the references to doxygen) from conf.py.

Yeah, for now lets do that.

2. The main thing you are looking for on this PR is the make target that generates the doxygen documentation (which is completed)

Yes

3. The only thing missing from this PR is @meshula's request to add a note to readthedocs on how to generate the doxygen documentation.

Yes, and I think you added this? When you feel like its ready for review, poke the ready for review button on the bottom bar.

Thank you for your contribution! And 2x checking but we have CLAs from you, correct?

[utsab]

Thank you @ssteinbach for answering our questions. We updated the PR as "ready for review"/

And 2x checking but we have CLAs from you, correct?

I sent in my signed CLA a couple days ago (on Feb 18). I have not received an official confirmation back from them yet, but I can reply back on this thread as soon as I receive the confirmation. @hisergiorojas has already received his CLA confirmation.

[jminor]

CLA for @utsab received ✔️
and we have one from @hisergiorojas as well.

[ssteinbach]

Looking good -- saw that there were some default fields in the dox config.

[ssteinbach]

Could add a TODO here to set this via the same mechanism in cmake that sets the version number in other places.

[ssteinbach]

you can use one of the logos in docs/_static/... maybe?

[utsab]

Thank you @ssteinbach for your feedback. We incorporated your suggestions in the latest commits.

[ssteinbach]

This is looking like a great start, thanks!

Two more really small requests -- I added a suggestion to have the makefile print out the location things get written to (to make it easy for a user to find it later), and turning quiet mode on. Quiet mode is still not that quiet because so many of our .h and .cpp files are missing documentation.

Do you happen to know if we can link the pages we have in docs into the doxygen documentation? That doesn't have to happen in this PR, just asking.

[utsab]

Do you happen to know if we can link the pages we have in docs into the doxygen documentation? That doesn't have to happen in this PR, just asking.

Thanks for your commit suggestions. I merged those in. About your question on linking the docs pages into the doyxgen documentation, I'm not sure at the moment, but I can try to do some research on this. To clarify. by "docs pages", do you mean linking to a url like this one?

https://opentimelineio.readthedocs.io/en/latest/tutorials/architecture.html

Also, could you give us an example to illustrate what you mean? I'm having a hard time understanding the use case for this.

[ssteinbach]

I meant, we have a bunch of markdown and rst files that readthedocs uses to build a page that are also checked into the repository, for example, the index is here:
https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/master/docs/index.rst

The markdown pages are here:
https://github.com/PixarAnimationStudios/OpenTimelineIO/tree/master/docs/tutorials

If there was an easy way of including that documentation in the doxygen documentation, that would be cool.

[meshula]

Sorry, I didn't mean to hit the Approve button, I was looking at a different PR. I can't see a way to undo it ~ I haven't reviewed this one!

[ssteinbach]

After some research, it looks like there are some bugs with Doxygen's support for markdown. Even so, I think that having the README.md as the mainpage.dox is better than nothing, and we can improve it over time. See this bug:

doxygen/doxygen#7688

I think the steps would be:

1. Add mainpage.dox (like in the bug above ^)
2. refer to README.md (using the way they note in the bug that works)

We can come back to this later and add references to more of the markdown documentation over time.

Thanks for sticking with this! This is a great addition.

[utsab]

@ssteinbach Thank you for clarifying. If I'm understanding you correctly, you would like to include all of the existing readthedocs documentation inside of doxygen's generated documentation. Thus, all of our documentation would be merged together in one place (in doxygen's web portal). Is this what you mean?

Based on your last comment, it sounds like you are recommending that we can at least start with including the README.md file. Sure, I'm happy to take a look at the bug report and implement its recommendation for how to add a reference to README.md in the mainpage.dox.

[ssteinbach]

@utsab not quite. It looks like there are bugs with the way that doxygen and Markdown interact. Despite that, the mainpage.dox (which generates the "index.html" page if you build the html version of the doxygen docs), would still be better with a reference to the README.md. The entire markdown documentation in the docs/ directory can wait until after doxygen fixes their markdown parsing bugs.

That would put the contents of the README.md (located in the top level directory of the repository) into the main page of the doxygen documentation.

[utsab]

@ssteinbach OK I think I understand now. For now, your request is to simply include a reference to README.md in mainpage.dox. Is this correct?

[ssteinbach]

Yup! Thanks @utsab!

[utsab]

Hi @ssteinbach, we started looking into the instructions on how to include a reference to README.md on the homepage of the generated doxygen documentation. We ran into some issues figuring out the instructions for mainpage.dox, but we did find another approach where we can set the USE_MDFILE_AS_MAINPAGE flag inside the dox_config file to point to README.md. This seems correctly bring in the README.md markdown. You can see the full code changes in the latest commit.

Is this a reasonable approach? If you prefer that we go with the mainpage.dox approach, we can continue investigating that direction as well.

[ssteinbach]

This looks great! Once this passes CI this looks ready to land to me. HUGE thanks for doing the legwork on this @hisergiorojas and @utsab!