On Jan 4, 2008, at 7:06 PM, Marshall Pierce wrote:
Over the past little while, I've been working here and there on
improving log4php to use PHP 5 features for my employer, and I've
gotten
the OK to contribute my improvements back to the project. I'm not
quite
finished with my improvements, but I wanted to get some feedback as I
continue forward.
Improvements:
- Relocate and rename classes to support autoloading (and provide an
autoload implementation)
- Move from php4 'var' to private/protected as appropriate and add
getters/setters as needed
- Bug fixes / other minor refactorings
- Use exceptions (still remaining to be done: make sure exceptions
will
not bubble up into code that uses log4php)
- Reformat code (mostly -- there are still parts that I haven't gotten
to yet) to match the standard we use at work
As far as code formatting goes, I'm just using what we use at work
(Allman style). I'm not particularly attached to it; that's just
what my
editor is set to. :)
The diff touches basically every single line of every file (except
chainsaw, which I didn't touch, and the original test code, which I
found unwieldy and didn't use), so I've attached my code as-is
instead.
I've included Knut Urdalen's unit tests, updated to work with my code.
I've also included a simple example usage file and config file.
Note that if you want to test the socket appender, you'll need
something
to connect to, so to save you the trouble of writing one, I've also
attached a very simple ruby script that listens on a socket and prints
out whatever it receives. Just run it with the port number to open a
socket on and configure the log4php xml file to use the proper IP
address and port.
No doubt I've caused at least as many new bugs as I've fixed, so
please
let me know if you find any.
Marshall Pierce
<socket_listener.rb><log4php.tar.bz2>
Thanks for your interest. I'm mentoring the incubation of log4php,
but not a PHP developer myself. I can't really express an informed
opinion on the code submission, but hopefully can help on some of the
process issues.
Incubation (that is, community building, sorting out legal issues,
etc) of log4php was restarted last summer after it was retired since
all of the original committers had faded away and the list had been
quiet for a long time. After a group of like-minded developers was
gathered, the Incubator and Logging PMCs voted to restart log4php,
there were a couple of commits and things went quiet again. That has
been fairly disappointing, maybe your submission will spark some
interest from the recently added committers to get things going again.
If you could see yourself as a regular contributor to the log4php, it
would be good to have a CLA on file (http://www.apache.org/licenses/#clas
). A signed CLA is required of all Apache committers and having a
signed CLA and a series of useful patches goes a long way to
establishing the track record necessary to support a vote for
developer access to the repo. If you don't see a continuing
involvement, that is you'd expect that you'd drop out of site after
this phase is over, it would be good to know that upfront too.
I understand how you end up with a change set that touches a lot of
lines and fixes a lot of things. Unfortunately, that makes it
difficult for any user or review to audit the code to see what changed
between versions or to back out parts if something is ill-conceived.
Part of the process that would be needed to accept this submission
would be to split out the changes into distinct concerns with
corresponding bug reports and then commit them individually.
I'd suggest creating a bug report for each of the entries in your list
of improvements at http://issues.apache.org/jira (look at the full
list of existing log4php issues to see if your concerns overlap, but
that list is short). Please describe the motivation, approach, etc or
anything backstory that would help a reviewer understand your
patches. If nobody else steps up, I'd be willing to commit patches
that have reasonable stories behind them and no objections. The bug
reports would still be beneficial even if you did not undertake
breaking up your submission along those lines and needed to let
someone else do that. If there are any other code changes that you've
been thinking about, it would be good to get a bug report logged even
if you don't have any code behind it.
It is undesirable to mix code reformatting with functional changes,
since it makes it difficult to uncover the functional changes. I'd
much rather have poor code formatting where I should follow the
evolution of the code than to have good code formatting. If code
reformatting is highly desirable, it would be best to do it
immediately after a release or immediately prior. If it is done, it
should be done in a commit that has no functional changes and clearly
labelled as a formatting only change.
