Some thoughts:
- use System_Folders to retrieve temporary folder
That's doable, but implementation in System_Folders is weaker - it just blindly assumes /tmp/ on Linux/OS X rather than querying the system (which sys_get_temp_dir() does properly).
- do not prefix your includes with PHPTAL_DIR (require_once
PHPTAL_DIR.'PHPTAL/FileSource.php';) this is handled by the system
configuration. In PEAR packages we just use require_once
'PHPTAL/FileSource.php';
This is needed to avoid mix-up of files from PEAR and other installations of PHPTAL, e.g.:
include "my/framework/libs/PHPTAL.php";
won't work properly without enforced path prefix. There's same problem when running test suite. Does PEAR have solution to this?
- use class constants instead of global statics.
- PHPTAL_XHTML and PHPTAL::XHTML are the same?!
This is in older/stable version for backwards compatibility. All global constants except PHPTAL_DIR are removed in upcoming version.
- put class properties on beginning of class
- public properties should not be prefixed with '__' (public $__file = false;)
Ok, I'll fix this.
- return values of methods are not documented
I've added lots of those in SVN.
- mention PHP version dependency in package.xml
ok.
- initial release of a package should not be "stable" but "beta".
I have linked to "PEAR-ified" alpha version in my previous post (I'll mark it beta once its accepted). You seem to have downloaded older version.
- initial release version should not be PHPTAL's but adhere to PEAR's
versioning scheme [1], e.g. 0.1.0
PHPTAL is not a new software. It has been distributed as PEAR package for years. I'm afraid that version reset could be confusing for existing users.
- there are no tests
There's plenty of phpunit tests in SVN. I don't include them in the package - should I?
https://svn.motion-twin.com/phptal/trunk/tests/
In general: Use phpcs [2] to check if your code adhers to PEAR's
coding standards.
I did fix many of those already, and result is visible in:
http://phptal.motion-twin.com/files/PHPTAL-1.2.0a5.tar.gz
PHP code sniffer is sometimes very obsessed about some IMHO trivial issues, e.g. this is a double ERROR:
substr($foo,0,1);
Some suggestions, like prefixing private variables, seem to be for PHP4. Am I really supposed to "fix" all of those?
Thanks for your comments.
--
regards, Kornel