Want To Add Doxygen Comments

Everything about development and the OpenMW source code.
Post Reply
User avatar
EmperorArthur
Posts: 33
Joined: 17 May 2014, 07:52

Want To Add Doxygen Comments

Post by EmperorArthur » 20 Aug 2014, 11:46

Hi folks,

I'd like to add some comments to the codebase, and change file headers to use the Doxygen format.

I know Zini likes code to be self documenting, but I think it can't hurt to have proper documentation for at least the classes and functions. Admittedly, there's a selfish reason for this as well. I've been following the project for a while now, but the main way I'm learning the codebase is by reading through the git pull requests. As I go through and slowly document everything you all can tell me when I'm being stupid and don't understand what something is or does.

This project of will touch almost every file, so I want to run it by here before dropping pull requests with lots of changes in your laps.

Digmaster
Posts: 22
Joined: 20 Apr 2014, 05:12

Re: Want To Add Doxygen Comments

Post by Digmaster » 20 Aug 2014, 16:52

I am massively in favor of this. I know that others aren't, because I've tried to go and add comments and have gotten complaints. But the fact of the matter is that or code is not always self commenting, and many functions are very confusing until you figure them out. Last I checked the documentation site wasn't updated since .28, of no one else is willing I'm more then happy to update it weekly or so

User avatar
EmperorArthur
Posts: 33
Joined: 17 May 2014, 07:52

Re: Want To Add Doxygen Comments

Post by EmperorArthur » 20 Aug 2014, 22:17

Right, so I've been working on creating a .mailmap file for git. To do that I pulled a list of everyone who has committed openmw code and the credits.txt file.

I found quite a few people that aren't listed in the credits.txt file, also someone is running git as root. Generic name is generic.

I couldn't find any of these names in credits.txt!

sirherrbatka <[email protected]>
sirherrbatka <[email protected]>
sergei <[email protected]>
riothamus <[email protected]>
nobrakal <[email protected]>
nkorslund <[email protected]>
mrcheko <[email protected]>
Miroslav Puda <[email protected]>
MiroslavR <[email protected]>
mckibbenta <[email protected]>
jeaye <[email protected]>
eroen <[email protected]>
eroen <[email protected]>
dreamer-dead <[email protected]>
crysthala <[email protected]>
Brother Brick <[email protected]>
Berulacks <[email protected]>
Axujen <[email protected]>

root <[email protected]>
unknown <[email protected](none)>

EDIT:
Removed full E-mail addresses to prevent spam bots from harvesting them
Last edited by EmperorArthur on 20 Aug 2014, 23:44, edited 1 time in total.

User avatar
psi29a
Posts: 4902
Joined: 29 Sep 2011, 10:13
Location: Belgium
Gitlab profile: https://gitlab.com/psi29a/
Contact:

Re: Want To Add Doxygen Comments

Post by psi29a » 20 Aug 2014, 22:24

Brother Brick is me.

Most of these should be self evident if you have followed your OpenMW history. ;)

Tarius
Posts: 574
Joined: 24 Oct 2011, 19:29

Re: Want To Add Doxygen Comments

Post by Tarius » 20 Aug 2014, 23:31

No, posting email like that to the forum is a good way for a passing bot(you dont need to have an account to read here) or something to scrape them up(whatever the technical term is) and add them to spam lists or what have you. Especially since stuff to do with MW has already landed spammers notice.
I am a bigger fan of tons of control rather than taking the "user-friendly" approach.
-Okulo

User avatar
EmperorArthur
Posts: 33
Joined: 17 May 2014, 07:52

Re: Want To Add Doxygen Comments

Post by EmperorArthur » 20 Aug 2014, 23:44

Thanks. The problem is just cross referencing multiple nicknames. That means that a simple forum search isn't going to work.

Out of 137 name/E-mail pairs, 19 weren't matched in the credits file.
File was generated using "git log --format='%aN <%aE>' | sort -u>.mailmap" then edited by hand to match the credits file.

Then use this beast to find who's on a file.
"git blame "$FILENAME" |sed -e 's/[a-z0-9]* (//' -e 's/).*//' -e 's/\s*[0-9].*//'|sort|uniq"

It's not super relevant right now, but it's interesting. I was considering putting the info on the Doxygen "@author" line of each file, but realized that would be stupid.

On the main topic.
If no one objects I'm going to start adding/changing the comments at the top of some files. Some of them have 2010 copyright info and refer to the old sourceforge page.

Chris
Posts: 1583
Joined: 04 Sep 2011, 08:33

Re: Want To Add Doxygen Comments

Post by Chris » 21 Aug 2014, 00:08

The comments are already doxygen-style for the most part, aren't they? The only annoying thing about them is that they're underneath the function declaration like

Code: Select all

void my_func();
///< Stuff
rather than

Code: Select all

/// Stuff
void my_func();
Cleaning up obviously-incorrect comments would be helpful, but I'd recommend discussing anything you may want to add new comments to, in case the interface may be too volatile to keep documented or in case someone else is working on the same code and your changes could cause merge conflicts.

User avatar
EmperorArthur
Posts: 33
Joined: 17 May 2014, 07:52

Re: Want To Add Doxygen Comments

Post by EmperorArthur » 21 Aug 2014, 02:08

Some of the functions are documented and some are not. Many of those functions that are documented don't have the full Doxygen \param and \return information. In some cases the data is there, but not in a Doxygen readable form.

The components/nif folder is a good example of somewhere that could use a bit of love, so I'll try to work on that part. I'll submit a pull request when I've handled a few files so you can see what I'm working towards.

User avatar
psi29a
Posts: 4902
Joined: 29 Sep 2011, 10:13
Location: Belgium
Gitlab profile: https://gitlab.com/psi29a/
Contact:

Re: Want To Add Doxygen Comments

Post by psi29a » 21 Aug 2014, 07:17

I hope you're not of the plane to attribute a file to a particular person. If you have to, better attribute the files to: OpenMW or OpenMW Team or something like that.

Also... some people didn't want to go in the credits file. Do a search in the forums, this topic has come up before.

User avatar
Zini
Posts: 5538
Joined: 06 Aug 2011, 15:16

Re: Want To Add Doxygen Comments

Post by Zini » 21 Aug 2014, 11:52

Okay. I sum up my position regarding doxygen comments:

- We are using them actively already.
- I repeatedly have encouraged people to use them.
- I use them on a regular basis, but there have been situations where I have been sloppy.

But I am not in favour of overdocumenting. In particular adding file-specific comments is pointless IMO (usually). Classes, functions, arguments and sometimes other types of stuff defined/declared inside a class can be worth a comment. In a few cases a file can also be included in that list (i.e. if it contains several highly coherent classes), but that should be the exception.
Also, you don't need to document every single function or argument, if these are completely obvious and there are no specifics that need to be considered. Please keep in mind that additional comments also require additional work to keep them in sync with the code. And comments that don't add any information that aren't immediately obvious from the code are just noise.

If you want to improve comments, I suggest to look for areas of the codebase with particularly poor comments. I am pretty sure that they are clustering up in certain places. The code base is decently big and maybe it would make sense to look for a tool that can calculate the comments per line ratio on a per-file basis. A low ratio does not necessarily mean there is a need for more comments, but it is a useful hint for where to look.

About the order (comments first then function or function first then comment):

Most of the later is from me. Since I got multiple requests from people to choose the other option, I have started to get used to it, but I still fall back into old habits on occasions.

You should generally avoid changing existing comments from the later form to the former, unless you are changing that comment anyway (when running under a version control system it is generally a good idea to avoid unnecessary changes).

For the record, I still hate comment first then function.

Post Reply