Skip to content

Conversation

jjjimenez100
Copy link
Contributor

Replaces all usages of log4j with the Slf4j API. This PR also removes references of log4j in pom and resource files.

@iluwatar
Copy link
Owner

@jjjimenez100 please resolve the conflict

@jjjimenez100
Copy link
Contributor Author

Fixed by bf2b3cb Although I'm not really sure that was the best way to fix the conflict since I did it on Ghub's UI. 😅

Copy link
Owner

@iluwatar iluwatar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good, but log4j is still present in several places. You can try to grep the codebase like grep -r -l "log4j" to see where it is still referenced.

@jjjimenez100
Copy link
Contributor Author

The changes look good, but log4j is still present in several places. You can try to grep the codebase like grep -r -l "log4j" to see where it is still referenced.

Ah right. About that, I think the only remaining are the ones under the naked-objects pattern. However, it seems like Apache ISIS' withLoggingAt(..); only accepts the concrete class of log4j's Level. Should I simply just remove that line?

@iluwatar
Copy link
Owner

It's ok for me to remove those statements. It would be also good to scan for System.out.println style logging. I think I've seen some of those in the codebase.
There are some conflicts now that should be resolved.

@jjjimenez100 jjjimenez100 force-pushed the #970-Single-logging-framework-should-be-enforced branch from bf2b3cb to 0a1cc7f Compare October 13, 2019 13:04
@jjjimenez100 jjjimenez100 force-pushed the #970-Single-logging-framework-should-be-enforced branch from 0a1cc7f to 1f34630 Compare October 13, 2019 13:08
@jjjimenez100
Copy link
Contributor Author

jjjimenez100 commented Oct 13, 2019

The only remnants of System.out... now would be on:

  • The Composite pattern
  • ConsoleLoggerModule

since the examples has test cases asserting the output from the console (which I believe the way the example was structured to begin with)

For the log4j ones, stuffs remaining are on persistor.properties and all of them are already commented.

Let me know if there are any other changes I should do. 😄
build success

@jjjimenez100 jjjimenez100 requested a review from iluwatar October 13, 2019 15:08
@iluwatar iluwatar merged commit cfdfedb into iluwatar:master Oct 13, 2019
@iluwatar
Copy link
Owner

Thanks @jjjimenez100 for the hard work you've done

@iluwatar iluwatar added this to the 1.22.0 milestone Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants