Code review comment for lp:~mhr3/libzeitgeist/symbols

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Nice! Some comments:

 1) I'd like to see a symbol_info_new() that takes ownership of the arguments since you introduce a lot of memory reallocation when building the symbol tree.

 2) Author in test-symbols.c is you, not me :-)

 3) I'd like to see get_children(), get_all_children(), and get_parents() as well. At least it is my experience that you quickly end up needing those once you start getting into it

 4) API documentation is needed for the public functions

 5) a VAPI update?

 6) Nice and thorough work on the unit tests!

review: Needs Fixing

« Back to merge proposal