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

Doxygen and texi documentation cleanup #1943

Merged
merged 14 commits into from
May 23, 2024

Conversation

gjanssens
Copy link
Member

Following a note by @jralls on a bug earlier this week, I have set out to clean up the documentation directory in engine.

As already suggested most of it is horribly out of date and no longer relevant. I have removed all of that. I did keep a few bits that seemed helpful still as they are explaining a few concepts in the code.

Other than that I have cleaned out the main doxygen page a bit.

Question: I'm tempted to eliminate the directory libgnucash/docs altogether.
The files that are still in there could move to other locations. Here are my proposals:

  • loans.txt and lots.txt could go into libgnucash/engine. They really document concepts from the engine code. We also have kvp_doc.txt there still.
  • callgrind.txt could perhaps be added to HACKING (after revision to today's build system)
  • python-bindings-doxygen.py can move to bindings/python
  • I'm not to sure what to do with the dia directory. The components diagram adds very little IMO. The other two more or less explain the relationships between transactions, splits, accounts and commodities. Those are developer oriented, but I don't know if they are worth keeping.
  • the other bits (doxygen's main file and config file, xml directory with DTD's, financial calculation html files) I'd move to [toplevel]/doc. Other than the xml directory they are not engine specific. I'd find it silly to keep only the xml directory in libgnucash/docs though, so IMO it can just as well live in [toplevel]/doc. But I'm open to better suggestions there.

Thoughts ?

@jralls
Copy link
Member

jralls commented May 20, 2024

@gjanssens Thanks very much for taking care of this! I'm in favor of deleting libgnucash/docs. Suggestions for the keeper files:

  • loans.txt, lots.txt, and kvp_doc.txt: Turn them into Doxygen comments in the relevant files.
  • callgrind.txt: adding it to Hacking is fine, but maybe the wiki would be better for both, with links in Readme.md.
  • python-bindings-doxygen.py is just Doxygen comments. Paste them into bindings/python/gnucash-core.py.
  • doxygen.cfg.in and doxygen_main_page.c to the root directory and the Doxygen build clause from the CMakeLists.txt to the top level CMakeLists.txt or extract it to a Doxygen-docs.cmake in common/cmake-modules and put the two files with it.
  • dia Render them to png and upload to the wiki?
  • The xml directory could move to libgnucash/backend/xml/DTD
  • constderv.html, finderv.html, and finutil.html to the wiki or the trash. Everything there is findable on the web, much of it in Wikipedia or Investopedia. There's no harm in putting it in our wiki, but IMO no real reason to either.

@fellen
Copy link
Member

fellen commented May 21, 2024

* dia Render them to png and upload to the wiki?

Perhaps dia is closer to svg than to png.
@derekatkins Is svg enabled in the wiki?

@@ -1,5 +1,6 @@
# CMakeLists.txt for libgnucash/backend/xml

add_subdirectory(DTD)
Copy link
Member

Choose a reason for hiding this comment

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

@gjanssens You forgot to add DTD to the dist-list at the bottom. That's why the ubuntu CI failed: It runs distcheck instead of check.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jralls yea, I saw that and have the local fix already in my tree. I didn't push yet as there were some extra tweaks to do.

gjanssens added 14 commits May 23, 2024 15:10
The file mostly had short descriptions that were very similar
to the descriptions of business objects it referred to. The
exception was an explanation of how the billing terms and
tax tables handled their immutable copies. That bit has been
moved to its own group under the Business group and is referenced
from the BillTerm and TaxTables group.

This commit also fixes rendering of the Address doxygen info
and expands the BillTerm info to explain how proximo due dates
work.
docygen_mainpage.c

None of them had useful information describing actual
implementation concepts or details.
It's extremely out of date and we no longer use it to keep track
of our design decisions and suggestions.

Only a section on KVP policy has been move into kvp_doc.txt
as it may make sense there.
Note: this also means the target directory changes from
      libgnucash/docs/html
      to
      [toplevel]/doxygen/html
Relevant diagrams have been added in the wiki as png files.
The html files with financial calculations are copyrighted
by someone outside of the gnucash project. I'd rather not
add that info to the wiki. Similar information should be
easy to find on the internet.
@code-gnucash-org code-gnucash-org merged commit e8e179f into Gnucash:stable May 23, 2024
4 checks passed
@gjanssens
Copy link
Member Author

@gjanssens Thanks very much for taking care of this! I'm in favor of deleting libgnucash/docs. Suggestions for the keeper files:

* loans.txt, lots.txt, and  kvp_doc.txt: Turn them into Doxygen comments in the relevant files.

I have done so for loans.txt, lots.txt and libgnucash/engine/TaxTableBillTermImmutability.txt
I'm holding off on kvp_doc.txt as it's slightly different from the others. It is set up as an index of all kvp values in use in the gnucash code. I don't think keeping track of this manually is very desirable. On the other hand I don't know if we have an easy way to list this data via clever grep-ing or regexing the code. I currently have no idea of how many kvp value we have defined by now and I think that's useful info to extract. So until we have come up with something workable I tend to keep this file as a reminder that this information is hard to track down.

* callgrind.txt: adding it to Hacking is fine, but maybe the wiki would be better for both, with links in Readme.md.

In the PR I moved it to Hacking, with the intention to eventually move it to the wiki. I forgot to do so in this PR, but have done so in a follow-up commit

* python-bindings-doxygen.py is just Doxygen comments. Paste them into bindings/python/gnucash-core.py.

Good idea. Done

* doxygen.cfg.in and doxygen_main_page.c  to the root directory and the Doxygen build clause from the CMakeLists.txt to the top level CMakeLists.txt or extract it to a Doxygen-docs.cmake in common/cmake-modules and put the two files with it.

I don't think they warrant their own cmake-module so I have moved the lines to the root CMakeLists.txt file

Note this will inevitably also change the output directory. So we'll need @derekatkins to update the nightly build script to read the built documentation from the new directory. It will now be built into [builddir]/doxygen/html instead of [builddir]/libgnucash/doc/html.

* dia Render them to png and upload to the wiki?

I did so for the two files that were still relevant. @fellen svg is not enabled in our wiki, so I went with png.

* The xml directory could move to libgnucash/backend/xml/DTD

Done, and fixed my initial distcheck failure.

* constderv.html, finderv.html, and finutil.html to the wiki or the trash. Everything there is findable on the web, much of it in Wikipedia or Investopedia. There's no harm in putting it in our wiki, but IMO no real reason to either.

I did not move these into the wiki. There were copyright notices in these files that made me suspect they were snatched from elsewhere long time ago. I'm not comfortable publishing this in our wiki. Additionally it would be more work than I care to do (and have time for) for information that is available online as you say.

@jralls
Copy link
Member

jralls commented May 23, 2024

@gjanssens Thanks again.

@gjanssens gjanssens deleted the doxygen branch May 25, 2024 12:32
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.

4 participants