» Details |
---|
|
» Comment |
Code is clean and well document, a few suggestions though: 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. ;-)) 2) I also propose phpsqs instead of sqs - maybe through "addInstallAs()". 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. 4) File structure in SVN should be ./pkgdir/Services/Amazon/ vs. ./pkgdir/Amazon 5) A general question/concern -- there's no Services_Amazon_SQS "hub" class, right? 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. Cheers, Till |