Re: Re: Working Version of Net_Monitor - Please Review :)

Hi Alan, OK, it makes sense to let the end user pass in an associative array. I have modified the package to take an optional third array of "general configuration" options -- including options to customize the subject line and the format of the alert messages. I have also added a sample script to demonstrate how to pass variables stored in an INI file to Net_Monitor: http://www.robertpeake.com/Net_Monitor/Net_Monitor-0.0.3/Net_Monitor-0.0.3/examples/ My next step is to complete the "Smart Alerts" feature. I have already included user-controllable configuration options to tell Net_Monitor where to write the file. Best, Robert Alan Knowles wrote:

It's better to seperate the configuration from the package, it allows the package to be used in conjunction with the end users prefered configuration method. - hence usually just a key/value array is for more flexible. It was kind of an early design mistake of dataobjects to depend on PEAR::getStaticProperty, rather than just implment a global ::setOptions(array $options) method or something. Regards Alan Robert Peake wrote:

Hi Alan, Thanks. I have addressed all the "minor" CS issues you mentioned, below. To address your request for a customizable subject line, provide for customizable error message formats, and to specify the logging file for "Smart Alerts" (which Bertrand Mansion thinks is a must-have feature) I am considering using an INI file and passing it in like so: class Net_Monitor { //...

    function Net_Monitor($config_file = 'Net/Monitor/config.ini.php') {
        //check file exists and is readable...
        parse_ini_file($config_file);

//... That way it will be easier to maintain BC with these features and account for a host of new features like file change and connection time monitoring. The alternative of overloading a bunch of default variables or using an associative array for flexibility seem less elegant than a simple INI file. Does the Pear community have any stance on using INI files of the form: ;<?php /* ; [foo] bar = baz ... I didn't see anything in the CS about external configuration files. Thanks for the constructive feedback so far. I am reluctant to move this to "proposal" yet as my understanding is that I can not edit the main document after it reaches this stage. Cheers, Robert Alan Knowles wrote:

Looks ok - a few minor things: return TRUE/FALSE should be return true/false PEAR_ERROR is PEAR_Error require() doesnt need brackets - getAlerter()/getClient() etc. File names should not have '_' in them | ||require_once 'Net/Monitor/Service/Generic.php'; rather than ||require_once 'Net/Monitor/Service_Generic.php';| I'd use Net_Monitor_Service, rather than Net_Monitor_Service_Generic as the base class. It would be nice to make the subject for the error email configurable (eg. "[System Failure] %s" ) so It's easy to filter and delete them. Regards Alan Robert Peake wrote:

Dear Pear Group and Pear Developers Mailing List, A working version of the Net_Monitor package is available from: http://pear.php.net/pepr/pepr-proposal-show.php?id=171 Please take a moment to review the proposal writeup, package documentation, and sample source file (browseable phps). If you are a voting member and see anything that would cause you *not* to vote for this package, please, please let me know. Other comments are welcome of course, but I am especially interested in comments from voting members at this point as I hope to move this up to proposal status soon. Thanks very much for your time. Best, Robert