Re: [PEPr] +1 for Web Services::Services_Amazon_SQS

On Mon, 2008-12-08 at 18:42 +0000, Till Klampaeckel wrote: > Till Klampaeckel (http://pear.php.net/user/till) has voted +1 on the > proposal for Web Services::Services_Amazon_SQS. > > Proposal information: > http://pear.php.net/pepr/pepr-proposal-show.php?id=582 > Vote information: > > http://pear.php.net/pepr/pepr-vote-show.php?id=582&handle=till > > This vote is conditional. The condition is: > > Code is clean and well document, a few suggestions though: Some updates on these suggestions now that voting is over: > > 1) The shebang in scripts/sqs should be "/usr/bin/env php" or a > replacement task @php_bin@ (vs. hardcoded /usr/bin/php). For example, > on most of the better OS' (e.g. Unix :-P) the php-cli is in > /usr/local/bin. ;-)) This has been fixed. > 3) Also, maybe add a .bat for windows? (phpsqs.bat) Or is there any > dependency which would not allow Windows users to run your code. I've added and tested this. > 5) A general question/concern -- there's no Services_Amazon_SQS "hub" > class, right? I've renamed Services_Amazon_SQS_Client as Services_Amazon_SQS. > I know that your "sqs" script basically implements all your functions, > but if I wanted to use your code straight (from another class), I'd be > in favour of using your classes vs. embedding exec() or system() in my > own code. > > Anyway, looking briefly at your code, I think I would need to copy > Services_Amazon_SQS_Application into my own library and work based on > that. (So it seems.) So instead of that, I'd propose you move > Services_Amazon_SQS_Application from "scripts/sqs" to > Services/Amazon/SQS.php and include it in "sqs" instead. > Reasons are a of course cleaner "sqs" app, and people can also easily > re-use your code for their own purposes. > 5 is sort of a RFC -- not conditional. I just thought I'd include it. > I've moved the class in the sqs script to Services_Amazon_SQS_CLI, which is in the regular directory structure. This does clean up the sqs script, although Services_Amazon_SQS_QueueManager should still be used instead for other applications. For Suggestion 2, I'd prefer to keep the CLI name as 'sqs' since it is not used by any other package (PHP or not). Thanks for your feedback, Mike