Uploaded image for project: 'Log4php'
  1. Log4php
  2. LOG4PHP-131

File appenders parameters

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 2.0.0
    • 2.1.0
    • Code
    • None

    Description

      While looking at issue LOG4PHP-130, i noticed that the three file appender classes had some strange voodoo in the setFile() method. This method currently takes one or two arguments, with the first being file name, and second being the append flag. I believe each setter method should set one and only one parameter. Append can be separately set via setAppend().

      The other thing I noticed, is that the mentioned appenders also have a setFileName() method which just takes the fileName param, without the second optional parameter. This seems a bit messy and inconsistent. Concerning the parameter name, it is internally called "fileName", but the name "file" is used in documentation and examples.

      To fix these, I made a patch in which I:
      1. Modified setFile() to remove method overolading. It now accepts only one argument: $file.
      2. Renamed the class member variable $fileName to $file so that it's consistent with the name of the parameter in docs and examples.
      3. Modified setFileName() to emit a warning that it is a deprecated function, then call setFile(). This method may be removed at some future version.
      4. Added some documentation and tidyed the code in a few places.

      Additionally:
      1. For consistency, added getters for params which had only setters (maxFileSize and maxBackupIndex in LoggerAppenderRollingFile)
      2. Since LoggerAppenderRollingFile::setMaximumFileSize() was already marked as deprecated, I modified it to trigger a warning when invoked.

      All the test pass without modification, however the deprecated warnings show as failures, so I modified tests to use setFile() instead of setFileName() and setMaxFileSize() instead of setMaximumFileSize().

      Just wanted to make sure everybody's ok with these changes before commiting them.

      Attachments

        1. file-appenders.patch
          11 kB
          Ivan Habunek

        Activity

          People

            juice Ivan Habunek
            juice Ivan Habunek
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: