Re: Re: [PEPr] Comment on Web Services::Services_Twilio

From: Date: Tue, 08 Mar 2011 13:40:00 +0000
Subject: Re: Re: [PEPr] Comment on Web Services::Services_Twilio
References: 1 2 3  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-54140@lists.php.net to get a copy of this message
Hi, we advocate code re-use heavily. Having said that HTTP_Request2 is not just a great abstraction to do HTTP calls, but also supports curl and sockets. Along with that it's also pretty easy to use a proxy with it. So it's best for your class to use it, but also add a setter so people can configure their own HTTP_Request2 instance (in case they are behind a proxy or want to use a different adapter etc.) and inject it. The CS stuff - I think I replied to you last night on IRC. The easiest is: pear install PHP_CodeSniffer cd /your/source/dir phpcs Services/* Btw, great use of mockery. :-) HTH, Till On Tue, Mar 8, 2011 at 2:21 AM, Neuman Vong <neuman.vong@gmail.com> wrote: > Hi Matthew, > > Thanks for your comments! We're working on shaping it up to PEAR standards. > I hope nobody minds too much that it's pushed out in pre-alpha form: we > thought it'd be best to get some feedback from the community as early as > possible. > > We thought about using HTTP_Request2, but that would create an extra > dependency. The current request class is really a thin cURL wrapper, so only > the curl extension is required. If that happens to fly directly in the face > of PEAR convention, please let me know and I'll have a talk with the team > here about additional deps. > > Thanks again! > > On Mon, Mar 7, 2011 at 6:14 PM, Matthew Fonda <mfonda@php.net> wrote: > >> >> Looks like a useful package! >> A couple comments after quickly looking through the code: >> >> 1. Make sure to follow PEAR coding standards >> 2. Add docblock comments >> 3. Instead of including your own HTTP request class with the package, >> consider using PEAR HTTP_Request2. >> >> -- >> http://pear.php.net/pepr/pepr-proposal-show.php?id=656 >> >

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