[PEPr] Comment on XMPP::JAXL
| From: | Christian Weiske | Date: | Tue, 04 Jan 2011 21:02:58 +0000 |
| Subject: | [PEPr] Comment on XMPP::JAXL | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-53937@lists.php.net to get a copy of this message | ||
I opened a "pearified" branch of my jaxl fork on github. It should serve as
example how to do it, and could server as starting point for you to get
jaxl prepared for PEAR:
https://github.com/cweiske/JAXL/tree/pearified
Please have a look at it.
- the compat directory is not full yet, I only added some examples so you
can see how it could look like
- I would split the package up into two packages: Net_XMPP and
Net_XMPP_Client. Net_XMPP only implements the xmpp protocol details, while
Net_XMPP_Client contains all the files that assists you in creating the
client.
Some things I saw when looking through the source code:
- Log -> replace with Log package
- no static logging, always use configured instance
- util -> replace curl() with http_request2 usage
- XEP classes without leading zeros, since 4 zeros may not be enough in the
future
- get rid of jaxl_require
- replace define() with class constants
- rename jaxl in the files to net_xmpp
- follow pear coding standards, run phpcs on it
- php5: public/private/protected instead of "var"
- use type hints in method parameters
- get rid of JAXL_BASE_PATH and jaxl_instance_count - no global constants,
no global variables allowed!
- I only fixed the file names and the "class $classname" declarations. I
did not test them, and did not rename class usages - this is left to do
- there are absolutely no unit tests, which is a shame for such a big bag
of source code
- docblocks everywhere are missing. very important is the description of
array keys that are required in the parameter arrays, i.e. in Net_XMPP_Get
- return values are also not documented
- parts of Net_XMPP_Client_Util probably can be replaced by using pear
packages, i.e. isWin() is available in the PEAR System class, hmacMD5 is in
the PEAR Crypt_HMAC package
--
http://pear.php.net/pepr/pepr-proposal-show.php?id=635