Vote Details for "PHPTAL" by gauthierm
| » Details |
|---|
|
| » Comment |
|
This looks to be a comprehensive and generally well written package; however, it was rushed to the voting stage. I think PHPTAL would be good to have in PEAR, but not in its current state. There are several outstanding issues that will prevent its adoption in PEAR. By applying these suggestions, the code will be more maintainable in the long term. 1.) phpcs, specifically - line length - whitespace, space around operators - required docblocks - required doc tags in docblocks 2.) immediately executable code is not allowed - PHPTAL - XML namespace code 3.) use require_once consistently instead of require 4.) specify optional extension dependencies in package.xml - gettext, specifically. Other extensions may also apply. 5.) closing php tags are required 6.) use class constants consistently instead of define() 7.) procedural code in Tales.php should be in a static class 8.) @package name is inconsistent between files 9.) there should be one class/interface per file 10.) Why do you re-implement an XML parser? SAX parsing is in PHP. Does the comment in the header still apply? 11.) SAX parser should not be named 'Dom'. DOM referrs to a different way of parsing. 12.) properties should be defined before methods in classes, not interlaced 13.) use PEAR packages to convert roman numerals PEAR::Numbers_Roman 14.) assertions may not be allowed in PEAR, a exceptions should probably be thrown instead 15.) complicated regexps like '/^[^$&\/=\'"><\s`\pM\pC\pZ\p{Pc}\p{Sk}]+$/u' should have a comment explaining what they do. Consider using /x and documenting the exression inline |