Alan Knowles [2004-04-23 03:12 UTC] can you add a link to the cvs page @ horde, I always get lost trying to find things there.
Martin Jansen [2004-04-23 07:48 UTC] I really like the package and the code.
Just one point: Can you perhaps include PEAR.php in SMS.php, so that people don't need to load it themselves?
Alan Knowles [2004-04-23 08:06 UTC] Thanks for the link
In general looks really good. - a couple of minor things.
* gettext _() , I'm not sure it's a good idea to force a requirement on gettext, I dont have any great ideas on alternatives though..
* getGatewayInfo/getGatewayParams = all look like convience methods, I'm not sure they add much value, other than making the API bigger.. (meaning more to document, more to read..)
* there's a few typos on the include/require - it should be 'Net/SMS/'.$class...
* Im not sure if the drivers should be (as in _ == / )
Net_SMS_ClickaTelViaSmtp
Net_SMS_ClickaTelViaHttp
or is it worth
- dropping the transport method??
- having Net_SMS_Http_ClickaTel.
Marko Djukic [2004-04-23 08:27 UTC] Some combined responses to the above:
- ok, putting the PEAR.php inclusion in SMS.php
- gettext - tough to do without it. The messages are essential to the end user. Without it they either a) end up with the send errors/status only in english or b) each implementation would have to jump hoops to get them localised. I'm using these in Italy so I've seen the significance of getting back meaningful localised messages to the end user. It can always be overcome by including a dummy gettext function for those that don't have it, and resulting in english only messages.
- getGatewayInfo/getGatewayParams are more than convenience, without them one would have to build an API to the API. Most gateways have significant varyiations in the level of capabilities and passable parameters. Having the drivers themselves handle their reporting allows for implementations to be very flexible. For example in Swoosh it is a snap to set up of several instances of the same gateway for different tasks/users. Passing this burden on to the implementation would seem like a bigger disadvantage compared to 20-30 lines of code in the driver.
- Typos ok thanks. Fixing them right now in the CVS.
- Driver names, sure. Renaming them would be fine. The transport would definately be needed. Clickatel has something like 5-6 different transports and I'd like to include a couple of them at least.
|
