Skip to content
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

reduce log levels of class import details #291

Merged
merged 1 commit into from
Jan 10, 2020
Merged

Conversation

codecholeric
Copy link
Collaborator

Since some users had trouble configuring their logging and consequently hanging Maven builds when using Windows CMD as their console, the log levels for the most detailed information during class file import is now reduced to TRACE. In the end it also seems reasonable to log details about field accesses and method calls on a different level than which classes are imported. And since meanwhile the logging of which classes are imported is DEBUG level and not INFO level anymore like originally, this is a reasonable adjustment.

Since some users had trouble configuring their logging and consequently hanging Maven builds when using Windows CMD as their console, the log levels for the most detailed information during class file import is now reduced to TRACE. In the end it also seems reasonable to log details about field accesses and method calls on a different level than which classes are imported. And since meanwhile the logging of which classes are imported is DEBUG level and not INFO level anymore like originally, this is a reasonable adjustment.

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
@ghost
Copy link

ghost commented Jan 9, 2020

DeepCode's analysis on #0c602a found:

  • 0 critical issues. ⚠️ 0 warnings and 0 minor issues. ✔️ 0 issues were fixed.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.


☺️ If you want to provide feedback on our bot, here is how to contact us.

@codecholeric codecholeric merged commit 5b050b4 into master Jan 10, 2020
@codecholeric codecholeric deleted the adjust-log-levels branch January 10, 2020 00:26
@hankem hankem mentioned this pull request Jan 31, 2020
codecholeric added a commit that referenced this pull request Feb 21, 2021
Since some users had trouble configuring their logging and consequently hanging Maven builds when using Windows CMD as their console, the log levels for the most detailed information during class file import is now reduced to TRACE. In the end it also seems reasonable to log details about field accesses and method calls on a different level than which classes are imported. And since meanwhile the logging of which classes are imported is DEBUG level and not INFO level anymore like originally, this is a reasonable adjustment.
@SebastianDietrich
Copy link

@codecholeric - imho the logging of which class is analyzed (line 104) should as well be reduced to TRACE. Then all class import infos would have the same level.

@codecholeric
Copy link
Collaborator Author

This was a conscious decision back then, because what classes are actually imported seems to me to be of a bigger relevance than all the details that are now logged on TRACE level. I.e. to see which classes are encountered is a different level of information than every fine grained bit of which method and annotation is imported with which details.

@SebastianDietrich
Copy link

I understand, but then again all these classes are "encountered" but not analyzed (only those defined via @AnalyzeClasses). Im my case (just starting a project) it lists 889 classes, of which only 58 really get analyzed. So I would suggest to reduce the logging to those classes that really get analyzed or at least correct the wording to "Encountering class ..."

@codecholeric
Copy link
Collaborator Author

I see, so you got mislead by the wording then. This debug statement is quite old, it was already there, before there was @AnalyzeClasses and at that time simply referred to "looking at class x" from the ClassFileImporter point of view. About only logging those classes, I'm not sure, because

a) this was also supposed to help in the case "the dependencies of my class behave strange, let's see if these classes are even imported". So it could also be confusing if dependencies are suddenly not listed anymore here
b) it would also increase complexity, since the same class is used in different places. That would mean the logging behavior needs to be parameterized, cluttering the method and making the code harder to understand (which is a minor argument, but means it needs a stronger case for changing it then if it would simplify things)

But of course it could log "processing class" (since it's the JavaClassProcessor) or "importing class" (since it's part of the import process) 🤷‍♂️ Then it would maybe clearer.

codecholeric added a commit that referenced this pull request Aug 22, 2021
as it seems to confuse some users, because it is unrelated to `@AnalyzeClasses` and only refers to the actual files the importer processes during the class import.

See #291 (comment)

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
@SebastianDietrich
Copy link

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants