[PEPr] +1 for Geo::Services_Openstreetmap

From: Date: Mon, 30 Jan 2012 17:11:09 +0000
Subject: [PEPr] +1 for Geo::Services_Openstreetmap
References: 1  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-54763@lists.php.net to get a copy of this message
Michael Gauthier (http://pear.php.net/user/gauthierm) has voted +1 on the proposal for Geo::Services_Openstreetmap. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=667 Vote information: http://pear.php.net/pepr/pepr-vote-show.php?id=667&handle=gauthierm Comment: Hi Ken, This package looks awesome. It must have been a lot of work to write. My vote is not conditional, but I have some feedback after reviewing the code: 1.) Why not Services_OpenStreetMap with CamelCase? 2.) Have a package-specific InvalidArgumentException subclass. 3.) Way::osmChangeXML() method name is confusing. Maybe Way::amendXML() or Way::amendChangeXML()? 4.) Class docs should have @throws tags where appropriate. 5.) @access tags should be removed since this is PHP5 code. 6.) Some files use a mix of under_scores and camelCase for variable names. (ex. Config.php). Should consistify this. I'd opt for camelCase. 7.) Some method names use Xml and some use XML. Should be consistified. 8.) Criterion::__construct needs better docs. I have no idea how to use it by reading the code. 9.) Package.xml says this requires 5.3. I couldn't see anything obvious that would prevent it from running in 5.2 10.) The custom autoloader should be optional. As far as I remember, PEAR1 code should still use require_once statements. If an autoloader is used, it should be optional in case projects are using their own PSR-0 capable autoloader. 11.) There are some very small issues that could be picked up with phpcs. 12.) I like how the package can support multiple API versions. 13.) Thanks for writing tests already!

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