-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: use logger instead of System.out.println
#5188
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logger changes seem fine to me. I will leave the LOGGER
vs logger
bit for others to decide upon.
I am a bit more concerned about the changes to the tools in the engine-tests/src/test/java/org/terasology/documentation
folder. I suspect that these tools may have been intentionally not using a logging framework.
engine-tests/src/test/java/org/terasology/documentation/BindingScraper.java
Show resolved
Hide resolved
af160ce
to
8f08b52
Compare
703bb14
to
59f865d
Compare
engine/src/main/java/org/terasology/engine/core/PathManager.java
Outdated
Show resolved
Hide resolved
engine-tests/src/test/java/org/terasology/documentation/apiScraper/ApiComparator.java
Show resolved
Hide resolved
59f865d
to
2ddc3c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ApiScraper
and ApiComparator
tools were introduced in #3253. As far as I can tell, they were never used and I am unsure about how to test them directly. The output appears to be intended as more human readable than machine readable, so the logger changes may not be significantly detrimental here. Either way, merging is is unlikely to break anything, since this code appears to be unused.
The rest of the changes raise no concerns for me.
I am approving this but would still prefer a second opinion before merging.
System.out.println
would it be acceptable to you to use logger instead of system out in these cases?