[PEPr] +1 for Web Services::Services_OAuthUploader

From: 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

« previous php.pear.dev (#53935) next »