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

On Sun, 2009-05-03 at 05:11 +0100, Kornel Lesiński wrote: > 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 ;) > Kornel, I'm happy enough with these changes to say my vote is no longer conditional. You should still strive to get the code to conform to phpcs, but I realize it is a larger code base and that is a LOT of work to finish. It can be worked on in smaller pieces at a later date. Anyhow, +1! Cheers, Mike