[PEPr] +1 for Web Services::Services_OAuthUploader
| From: | Till Klampaeckel | Date: | Tue, 04 Jan 2011 16:42:31 +0000 |
| Subject: | [PEPr] +1 for Web Services::Services_OAuthUploader | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-53935@lists.php.net to get a copy of this message | ||
Till Klampaeckel (http://pear.php.net/user/till) has voted +1 on the proposal for Web
Services::Services_OAuthUploader.
Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=650
Vote information:
http://pear.php.net/pepr/pepr-vote-show.php?id=650&handle=till
This vote is conditional. The condition is:
Looks great so far, also kudos on the tests.
My additions/requests which make this conditional:
1) Always use type hinting (e.g. for the consumer, etc.) (nice to have)
2) Your class names are off, e.g. class Services_TwippleUploader should be
Services_OAuthUploader_TwippleUploader (required per PEAR cs)
E.g. pear package-validate:
Warning: in YfrogTest.php: class "Services_YfrogUploaderTest" not prefixed with package
name "Services_OAuthUploader"
Warning: in TwitpicTest.php: class "Services_TwitpicUploaderTest" not prefixed with
package name "Services_OAuthUploader"
Warning: in TwitgooTest.php: class "Services_TwitgooUploaderTest" not prefixed with
package name "Services_OAuthUploader"
Warning: in TwippleTest.php: class "Services_TwippleUploaderTest" not prefixed with
package name "Services_OAuthUploader"
Warning: in PlixiTest.php: class "Services_PlixiUploaderTest" not prefixed with package
name "Services_OAuthUploader"
Warning: in ImglyTest.php: class "Services_ImglyUploaderTest" not prefixed with package
name "Services_OAuthUploader"
Warning: in YfrogUploader.php: class "Services_YfrogUploader" not prefixed with package
name "Services_OAuthUploader"
Warning: in TwitpicUploader.php: class "Services_TwitpicUploader" not prefixed with
package name "Services_OAuthUploader"
Warning: in TwitgooUploader.php: class "Services_TwitgooUploader" not prefixed with
package name "Services_OAuthUploader"
Warning: in TwippleUploader.php: class "Services_TwippleUploader" not prefixed with
package name "Services_OAuthUploader"
Warning: in PlixiUploader.php: class "Services_PlixiUploader" not prefixed with package
name "Services_OAuthUploader"
Warning: in ImglyUploader.php: class "Services_ImglyUploader" not prefixed with package
name "Services_OAuthUploader"
3) If a __construct doesn't do anything but call parent::__construct(), you don't need to
implement it at all. (nice to have ;-))
4) fix your package.xml:
sudo pear install package.xml
ERROR: file ./Services/OAuthUploader/Exception.php does not exist
I'm guessing this is because the files are in 'src', I just added <dir
name="src"> around it and it seemed to have worked, but then the problem is that it
installs into "php_dir/src/YOURFILES".
The workaround (and anyone please correct me if I am wrong) is either to drop the "src"
directory completely or to transform the paths on install.
5) Instead of hard-coding "0.1.0" into the files, use "Release:
@package_version@" and add a replacement rule to package.xml (nice to have):
http://pear.php.net/manual/en/guide.developers.package2.tasks.php
6) Improve tests: Down the road, I'd add a config with credentials for the services. If no
config file/credentials is/are provided for a service, skip the test (instead of failing it). In
this regard -- it might also make sense to mock the services so the tests always run no matter if
something is down or not. (nice to have)
Anyway, this looks really great! Looking forward to having it in PEAR.
Till