GNOME Bugzilla – Bug 754764
Budget Doxygen documentation - gnc-budget-view.c
Last modified: 2018-06-29 23:42:58 UTC
Trying to trace down bugs in budgeting has been difficult due to insufficient documentation on how the code works (i.e. the doxygen generated documentation). This bug is so that I can get the code documented better and hopefully contribute in a useful way.
Created attachment 310959 [details] [review] Doxygen Comments inserted into gnc-budget-view.c I am a noob and this is my first patch, so hopefully I have done it right. I tried to branch of maint, but I am not sure that github desktop did this properly (working on windows right now). I am doing this documenting to help improve my understanding of how gnucash works, as well as to hopefully help others. Please let me know if I have got anything wrong!
Comment on attachment 310959 [details] [review] Doxygen Comments inserted into gnc-budget-view.c Hi Matt, Thanks for your first patch! It's technically fine (applies cleanly and builds without errors) so I have committed it. I'll note that the subject line of your patch said "2/2", and there was only one patch. Did you intend to send 2 patches (also the "1/2") ? There were a couple of minor issues with it on other levels. Nothing serious. Most of them I have already fixed in a follow up commit. I'll mention them here so you can keep them in mind for future patches. 1. coding style: the source files are indented using spaces instead of tabs. It appears your editor on the other hand automatically uses tabs. Can you alter the preferences of your editor to use spaces for indentation as well ? Gnucash typically uses 4 spaces per tab. I corrected this for your patch in the follow up commit. 2. I note you have also added doxygen comments for static functions. I don't think those will appear in the generated documentation, so you may as well change those in to ordinary comments in the source file. The reason is that static functions are not publicly accessible and doxygen is meant to document the public interfaces in the source files. Hence static functions are ignored by doxygen. It's ok to add an internal comment block for information purposes in the source file itself. 3. I noticed you have edited the top comment block to insert doxygen data. Personally I'd leave the top comment untouched and add a separate block for the doxygen header right underneath it. I haven't changed this for your patch, because I'm not sure what the general preference is here. 4. I also noticed you have double doxygen information for gnc_budget_view_new. This is defined once in gnc-budget-view.h and once in gnc-budget-view.c. Choose one, not both please. 5. I filled in a few of the missing fields for you in the GncBudgetViewPrivate struct :) I'm looking forward to your next additions.
I wasn't sure if I should close this bug now. Do you intend to provide additional doxygen patches for gnc-budget-view ? If so, you can add them here. If not, feel free to close this bug now.
Thanks for the tips Geert, that does help. I'll close this bug now, opening new ones for any other files I get a chance to comment. The second patch you talk about there was automatically generated by git due to my merge/sync status at the time - so not relevant. Thanks again! Matt
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=754764. Please update any external references or bookmarks.