log4j support #91

Closed
quintonm opened this Issue Oct 12, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@quintonm
Member

quintonm commented Oct 12, 2013

Migrated from sourceforge: https://sourceforge.net/p/p6spy/feature-requests/4/

add log4j support for the log files to take advantage
of all of logging options log4j offers

@typekpb

This comment has been minimized.

Show comment
Hide comment
@typekpb

typekpb Nov 5, 2013

Member

Log4j support is already implemented, this one should track a need to add proper junit tests.

As an extra idea, how about deprecating/not supporting options:

log4j.<property_name>

any more? As whoever is using log4j in his/her app would have own log4j config provided already and who wants can still configure log4j outside of p6spy properly.

I think we should focus on p6spy configuration, and keep log4j stuff as separate as possible.

Would that make sense?

Member

typekpb commented Nov 5, 2013

Log4j support is already implemented, this one should track a need to add proper junit tests.

As an extra idea, how about deprecating/not supporting options:

log4j.<property_name>

any more? As whoever is using log4j in his/her app would have own log4j config provided already and who wants can still configure log4j outside of p6spy properly.

I think we should focus on p6spy configuration, and keep log4j stuff as separate as possible.

Would that make sense?

@quintonm

This comment has been minimized.

Show comment
Hide comment
@quintonm

quintonm Nov 5, 2013

Member

Agreed. We should deprecate the log4j appender.

Member

quintonm commented Nov 5, 2013

Agreed. We should deprecate the log4j appender.

@typekpb

This comment has been minimized.

Show comment
Hide comment
@typekpb

typekpb Nov 6, 2013

Member

should be solved along with: #131

Member

typekpb commented Nov 6, 2013

should be solved along with: #131

@typekpb

This comment has been minimized.

Show comment
Hide comment
@typekpb

typekpb Dec 9, 2013

Member

@quintonm as you already implemented: a7b3e48
would it make sence to completely remove the Log4jLogger, as well as the deprecated configuration (log4j.*)?
I think it causes us just trouble ( #170 ) and I'm not too much motivated to invest to something we plan to remove anyway.

We could close this one, as well as #170.

Member

typekpb commented Dec 9, 2013

@quintonm as you already implemented: a7b3e48
would it make sence to completely remove the Log4jLogger, as well as the deprecated configuration (log4j.*)?
I think it causes us just trouble ( #170 ) and I'm not too much motivated to invest to something we plan to remove anyway.

We could close this one, as well as #170.

@quintonm

This comment has been minimized.

Show comment
Hide comment
@quintonm

quintonm Dec 10, 2013

Member

Yes, I think that would make sense.

On Mon, Dec 9, 2013 at 9:24 AM, Peter Butkovic notifications@github.comwrote:

@quintonm https://github.com/quintonm as you already implemented:
a7b3e48a7b3e48
would it make sence to completely remove the Log4jLogger, as well as the
deprecated configuration (log4j.*)?
I think it causes us just trouble ( #170#170) and I'm not too much motivated to invest to something we plan to remove
anyway.

We could close this one, as well as #170#170
.


Reply to this email directly or view it on GitHubhttps://github.com/p6spy/p6spy/issues/91#issuecomment-30140450
.

Member

quintonm commented Dec 10, 2013

Yes, I think that would make sense.

On Mon, Dec 9, 2013 at 9:24 AM, Peter Butkovic notifications@github.comwrote:

@quintonm https://github.com/quintonm as you already implemented:
a7b3e48a7b3e48
would it make sence to completely remove the Log4jLogger, as well as the
deprecated configuration (log4j.*)?
I think it causes us just trouble ( #170#170) and I'm not too much motivated to invest to something we plan to remove
anyway.

We could close this one, as well as #170#170
.


Reply to this email directly or view it on GitHubhttps://github.com/p6spy/p6spy/issues/91#issuecomment-30140450
.

@ghost ghost assigned typekpb Dec 12, 2013

@typekpb typekpb closed this in 47f31c0 Dec 12, 2013

@typekpb typekpb added this to the 2.0.0 milestone Feb 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment