You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4php-dev@logging.apache.org by "Ivan Habunek (JIRA)" <ji...@apache.org> on 2010/11/20 12:08:13 UTC
[jira] Created: (LOG4PHP-131) File appenders parameters
File appenders parameters
-------------------------
Key: LOG4PHP-131
URL: https://issues.apache.org/jira/browse/LOG4PHP-131
Project: Log4php
Issue Type: Improvement
Components: Code
Affects Versions: 2.0
Reporter: Ivan Habunek
Assignee: Ivan Habunek
Fix For: 2.1
Attachments: file-appenders.patch
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.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (LOG4PHP-131) File appenders parameters
Posted by "Ivan Habunek (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LOG4PHP-131?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ivan Habunek updated LOG4PHP-131:
---------------------------------
Attachment: file-appenders.patch
> File appenders parameters
> -------------------------
>
> Key: LOG4PHP-131
> URL: https://issues.apache.org/jira/browse/LOG4PHP-131
> Project: Log4php
> Issue Type: Improvement
> Components: Code
> Affects Versions: 2.0
> Reporter: Ivan Habunek
> Assignee: Ivan Habunek
> Fix For: 2.1
>
> Attachments: file-appenders.patch
>
>
> 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.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (LOG4PHP-131) File appenders parameters
Posted by "Ivan Habunek (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LOG4PHP-131?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ivan Habunek resolved LOG4PHP-131.
----------------------------------
Resolution: Fixed
Committed in revision 1059522.
> File appenders parameters
> -------------------------
>
> Key: LOG4PHP-131
> URL: https://issues.apache.org/jira/browse/LOG4PHP-131
> Project: Log4php
> Issue Type: Improvement
> Components: Code
> Affects Versions: 2.0
> Reporter: Ivan Habunek
> Assignee: Ivan Habunek
> Fix For: 2.1
>
> Attachments: file-appenders.patch
>
>
> 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.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.