Re: [PEPr] +1 for HTML::PHPTAL

On 28.04.2009, at 20:16, Till Klampaeckel wrote:

Till Klampaeckel (http://pear.php.net/user/till) has voted +1 on the proposal for HTML::PHPTAL. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=597 Vote information: http://pear.php.net/pepr/pepr-vote-show.php?id=597&handle=till This vote is conditional. The condition is: All reasons have been stated in previous comments and by Michael.

I think I've fixed these issues: 2, 3, 4, 5, 6, 7, 8, 11, 12, 14, 15 http://phptal.org/latest-pear.tar.gz 1.) phpcs, specifically - line length - whitespace, space around operators - required docblocks - required doc tags in docblocks I've went trough code and fixed all important problems that I've noticed. I've also fixed "low-hanging fruit" that I could safely do with regular expressions. However number of issues that phpcs reports is daunting. Many of them are trivial, but require manual fixing. I appreciate neatly formatted and annotated code, but I've got a feeling that amout of time I need to fix all issues is far greater than amount of time this could save other maintainers. 9.) there should be one class/interface per file I've split all files except where classes were very small and logically grouped together (e.g. exceptions). I'm reluctant to split files further, because it would create numerous files that contain very little code, make codebase look larger and less approachable, and get in the way of finding classes that are actually important. I understand that situation is going to be different in PEAR2. I'd like to keep classes grouped until new standard is used. 10.) Why do you re-implement an XML parser? SAX parsing is in PHP. Current parser meets requirement of allowing micromanagement of XML and offers graceful migration path for existing ill-formed templates. I'm not going to switch parser in this release. 13.) use PEAR packages to convert roman numerals PEAR::Numbers_Roman Current code is already written, tested and doesn't take much. I don't see any benefit in replacing it with a package, unless someone finds bug in current code or Romans change their numerals ;) -- regards, Kornel