[mythtv] first contribution
Nigel Pearson
nigel at ind.tansu.com.au
Thu Sep 20 03:02:51 UTC 2007
Thanks for your patch. Feel free to do some more!
(i.e. understanding & documenting all
of the other nasty classes :-)
> I guess the last step is feedback?
Daniel has grabbed the ticket, so he may give you some
before he commits it, but here are my trivial comments:
1) Some whitespace changes slipped in there. e.g.
- MythThemedMenuPrivate(MythThemedMenu *lparent, const char *cdir,
+ /**
+ * @param lparent menu that owns this instance
+ * @param cdir directory where theme is stored
+ * @param lstate corresponding settings of the theme
+ */
+ MythThemedMenuPrivate(MythThemedMenu *lparent, const char *cdir,
2) Some of the brief descriptions
are too long to be, er, "brief"?
I reckon if the brief is more than about 60 chars,
then it needs to be split into a brief,
and a more detailed paragraph.
In the generated Dox, this will be split between
the function summaries (the brief),
and the more detailed "Member Function Documentation"
(everything else)
3) A few of the comment blocks should
probably have a brief tag? e.g.:
/**
* Set up UI according to the corresponding mythThemedMenuState
*/
void SetupUITypes();
4) Some of the descriptions are a tiny-bit obvious,
and could maybe be removed? e.g. this one:
/**
* @return true if the theme has been found
*/
bool foundTheme(void);
--
Nigel Pearson, nigel at ind.tansu.com.au| "They did the
Telstra Net. Eng., Sydney, Australia | white man power dance,
Office: 9202 3900 Fax: 9261 3912 | and ... Shazam"
Mobile: 0408 664435 Home: 9792 6998 | Lois Lane - Smallville
More information about the mythtv-dev
mailing list