Cool. I'll have another look at the code today and fix the phpdoc. And I will start writing a guide on developing log4php components in the wiki at some point.
Regards, Ivan On 26 March 2012 08:17, Florian Semm <[email protected]> wrote: > Am 25.03.2012 10:16, schrieb Ivan Habunek: > > Hi everybody, especially Florian, > > I think this is a great opportunity to lay down some rules for > designing appenders. Here are several comments I have on the FirePHP > appender implementation: > > 1. Check for pre-requisites on activation. > > You should check for presence of FirePHP library in activateOptions(). > This method is called before any logging happens and is the perfect > place for this. If pre-requisites are not met, then close the appender > by setting $this->closed = true. This way no further logging will be > forwarded to this appender. This is better than checking on each > append. > > done. > > > 2. Don't fail silently. > > This is very important: if it doesn't work, users should be informed > that it doesn't work, and if possible why it doesn't work. Appenders > have a warn($msg) method for this purpose. If you can't find the > firephp lib, warn the user that this is the problem. > > For an example of 1 and 2 see activateOptions() in LoggerAppenderPDO:126. > > done. > > > 3. Use a layout if appropriate. > > I don't see why we should not use a layout in this appender. It would > allow the user to define his own format instead of forcing our own in > formatMessage(). > Bruce mentioned in his comment (http://bit.ly/H4jbhc) that firephp can > render arrays and objects, so perhaps we can make it so that the > layout is not used for those types? To discuss. > > done. > > 4. Always add documentation along with a new appender > > That means, create a new page for the appender at: > /log4php/src/site/xdoc/docs/appenders/firephp.xml > (easiest way is to copy and modify an existing appender page) > > In this page, list the configurable parameters, prerequisites and a > simple example. > > Then add the page to the menu in: > /log4php/src/site/site.xml > > done. > > > 5. Add to XML change log > > Every time a non-trivial change is made to the code, an entry should > exist in the change log: > /log4php/src/changes/changes.xml > > Here is a reference file for the changes xml: > http://maven.apache.org/plugins/maven-changes-plugin/changes.html > > Basically, for this change type is "add", due-to is Bruce, and dev is you. > > > done. > > 6. Add to changelog page > > Big changes such as new features, any breaking changes, etc. should be > included in the changelog page: > /log4php/src/site/xdoc/changelog.xml > > Have a look at how it was done for previous versions: > http://logging.apache.org/log4php/changelog.html > > done. > > > 7. We should agree on which phpdocs tags to include. > > There seems to be an abundance of phpdoc tags used in this appender, > some of which are used wrongly, and some of which are not needed. Not > to go through them, one by one, I will make a little guide for phpdoc. > > hopefully done. > > > 8. Conclusion: > > So this turned out to be a long email... > > I wrote all these issues down (instead of just fixing them myself) > because I want to show you what is required before the new code can be > released. Unfortunately, there is more work than just writing code. > And some of that work is boring. I should know. :-) > > If you need any further with this, just ask. > > It would be helpful to document all this steps/requirements. > > Regards, > Ivan > > > regards, > > Florian
