kolibri-discuss team mailing list archive

[Anders Steinlein <anders@xxxxxxx>]

This looks very good, especially with the new getDom() together with the XslTransformer changes, and the streamlining of element names XmlGenerator creates. I'm about to have a closer look and do some more performance testing, but have a few nitpick comments after a quick read-through:

- The documentation on append() is wrong as it relates to scalar values and the $container parameter.
- getDom() and getXml() misses general documentation. "@return DOMDocument", etc, is then enough for the return tag.
- I still miss some comments on buildComplex(). Especially the test on line 125, where the preceding comment says *what* the code does, but not *why*. I miss some documentation on what really determines our element names, without having to fully understand the internals of XmlGenerator. Maybe an introduction in the class documentation?

Even more minor nitpicks I mention mostly for us to remember going forward:
- Line opp parameter comments, i.e. in buildScalar(). Makes it easier to read.
- Use spaces when lining up parameter comments. This lets people use whatever tab-size they like without messing up alignment.
- Try to keep line lengths under 95 chars. :)
-- 
https://code.launchpad.net/~frode-danielsen/kolibri/xml-generator-rewrite/+merge/5284
Your team Kolibri Discuss is subscribed to branch lp:kolibri.