Kornel Lesinski [2009-04-03 09:59 UTC] PHPTAL has been distributed as pear package for a long time.
You might want to look at SVN repository:
https://svn.motion-twin.com/phptal/trunk
and upcoming release (there is very stable 1.1 branch):
http://phptal.motion-twin.com/files/PHPTAL-1.2.0a4.tar.gz
Till Klampaeckel [2009-04-19 18:18 UTC] 2009/4/19 Kornel Lesiński <kornel@aardvarkmedia.co.uk>:
>> 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).
If that is the case, I'd provide a patch to System_Folders and switch to it when it's fixed.
> (...)
>> - 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.
Maybe wrap it into a phing task and remove the constants for the release on PEAR. I don't think there's a requirement to keep BC with non-PEAR versions, I could be wrong though. Since this will be your first release on PEAR you get a clean start.
>> - 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.
You should go easy on the version. Once you go stable, BC breaks are not allowed. There are a couple examples where developers raced to 1.0 and had to start a v2 package already because of a bugfix or feature request that resulted in a BC break. If you give it some time, the code can mature.
> There's plenty of phpunit tests in SVN. I don't include them in the package
> - should I?
Yes, you should -- mark them as tests in your package.xml.
>> 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?
The idea is to follow the CS 100%. We frequently have this discussion and while it's annoying at first, it helps a lot of people in the long run. E.g. the benefits are that developers who are familiar with PEAR code, will have it easier to work with your code as well. Or maybe down the road someone else takes over maintainership of your package, etc. p.p..
Till
Kornel Lesinski [2009-04-20 16:39 UTC] Updated package with (some of) the changes suggested in comments:
http://phptal.motion-twin.com/files/PHPTAL-1.2.0a7.tar.gz
|
