Re: [PEPr] Call for votes on Web Services::Services_IP2Country
| From: | Samuel ROZE - D-Sites | Date: | Thu, 20 Aug 2009 19:00:50 +0000 |
| Subject: | Re: [PEPr] Call for votes on Web Services::Services_IP2Country | ||
| References: | 1 2 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-52703@lists.php.net to get a copy of this message | ||
Hi, :)
Thank you for you reply.
Le jeudi 20 août 2009 à 20:29 +0200, David Jean Louis a écrit :
> Samuel ROZE a écrit :
> > Samuel ROZE (http://pear.php.net/user/rozesamuel) has initiated the call for votes on Web
> > Services::Services_IP2Country.
> >
> > Please review the proposal and give your vote here:
> > http://pear.php.net/pepr/pepr-proposal-show.php?id=608
> >
>
> Hi Samuel,
>
> Before the vote I'd like to add some comments to the excellent ones you
> got. (I'm commenting here cause now comments are closed.)
>
> The tarball gives a 404, so I'll speak about the code here:
>
> http://tasks.d-sites.com/repositories/browse/librairies/Services_IP2Country
>
> 1. the service URL (http://i2c.mes-stats.fr/get?ip=) seems to be
> provided/hosted by you, am I right ? do you think it will be something
> "durable", I mean will you host the service as long as you can ? It
> would be too bad if the service was down 3 monthes later;
It is. It'll be online for years because it is used by "Mes-Stats", my
project which will be my business. ;-)
>
> 2. your package layout should be something like:
> docs/
> |_ README
> |_ LICENSE
> examples/
> |_ uses.php
> tests/
> Services/
> |_ IP2Country.php
> |_ IP2Country/
> |_ Exception.php
> |_ Transport.php (some Services_IP2Country_Transport interface
> or abstract class)
> |_ Transport/
> |_ HTTP.php (class Services_IP2Country_Transport_HTTP)
> |_ SOAP.php (class Services_IP2Country_Transport_SOAP)
> package.xml
For you, this package is an "I2C" package. But no. Of course, there's
I2C "adapter" because it's my own project, and Services_IP2Country was
created for its BUT it allows new adapters for another IP-to-Country
service.
>
> (Currently you have all the code in the same directory and there is no
> package.xml.);
In fact, I want to make this file when (and if) the package is released.
Because, you will send me feedback and i'll change my library ;-)
>
> 3. speaking of Transports, beware, not everybody has "allow_url_fopen"
> enabled, maybe you could throw an exception explaining this, or use
> HTTP_Request2 as a dependency;
>
> 4. you should provide some unit tests (this can come later but it would
> be great if you provided some quickly).
>
> Code is very clean otherwise.
>
> --
> David
>