- In _store(), is !preg_match('/^http:\/\/([a-z0-9.-]*):([0-9]*)\/(.*)$/' really correct? That would match http://...:00/ (btw use ! as regex delimiter so you can use / without the need of escaping it.)

- I don't think the require_once 'PEAR.php'; is needed for the exceptions

- Regarding Philippe's statement "The open { for a method declaration is on the following line not on the same as the "function" keyword", the documentation clearly states to use K&R, which means putting the { on it's own line (In other words, ignore what Philippe said about that as what he said goes against the CS).

If PEAR.php does indeed end up being needed by the exception (as the code comment says), maybe the exception class file would be a better place to put the require_once('PEAR.php') line.

Maybe rename private function request() to _request(), to follow the naming convention of your other private function _store(). That probably means I should also hint at renaming your private vars $_socket and $_domain. Actually, why are the two timeout vars public?

Sounds like we're giving you lots of nitpick-the-details comments, which probably means the overall package idea is good, its API looks fine, and we reviewers are left to just be human Codesniffers :)

- Chuck Burgess"Check your license tags... they link to BSD, but docblock text says PHP license." I didn't see "PHP" in the license tag, but I was missing the license name. I added "BSD" as the license name.

- Removed @access, as phpdoc picks it up from the code. I'll put it back in if you all prefer it.

- Philippe Jausions: "Why use microseconds as timeouts if they're going to be divided by 1000000?" There are two timeouts, one passed to a function that takes float seconds and the other to a function that takes separate seconds and microseconds args. I want to be more consistent than that, so I chose microseconds. If I change it to seconds, then somebody would be right to ask, "Why seconds, if you're just going to multiply by 100000?"

I see the change to the LICENSE text in Mogile's file-level docblock to show BSD, matching its @license tag, but the Exception file is still showing "3.0 of the PHP license" paired with the @license tag showing BSD. I'm guessing you want to change that LICENSE text to BSD also.

I keep bringing up the license clarifications only because I've seen how hard it can be to change/correct them once a package gets released.

- Added File_Mogile::$curlTimeout, as suggested by Dormando on the MogileFS email list. (Since CURL_TIMEOUT is in seconds, this made two of three timeout configs that are natively seconds, so I made them all seconds for consistency.)

"- Added File_Mogile::$curlTimeout, as suggested by Dormando on the MogileFS email list. (Since CURL_TIMEOUT is in seconds, this made two of three timeout configs that are natively seconds, so I made them all seconds for consistency.)"

I'd make that something more generic as you may some day wish to switch from curl to some other library (HTTP_Request, HttpRequest, etc.).