-
Notifications
You must be signed in to change notification settings - Fork 33
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
#646: Add variable for intellij to disable console log output #729
Conversation
Pull Request Test Coverage Report for Build 11963975770Details
💛 - Coveralls |
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.
@KianRolf thanks for your addition of the IDE_ENABLE_LOG variable. I've added some CRs, please resolve.
cli/src/main/java/com/devonfw/tools/ide/variable/IdeVariables.java
Outdated
Show resolved
Hide resolved
…java Co-authored-by: jan-vcapgemini <[email protected]>
Co-authored-by: jan-vcapgemini <[email protected]>
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.
Thanks for addressing the change requests. I think we should also add an entry to our documentation, so the user becomes aware of this feature.
I'm not sure about the naming yet. IDE_ENABLE_LOG could be misinterpreted that the logging is now being done into a file. Maybe we need 2 variables instead? IDE_ENABLE_FILE_LOG (logs intellij errors into log file) and IDE_ENABLE_LOG (disables complete logging) |
I think renaming the variable IDE_ENABLE_CONSOLE_LOG avoids the confusion and i also added IDE_ENABLE_CONSOLE_LOG to variables.adoc in documentation. |
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.
Nice solution for the naming. Ready for review.
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.
@KianRolf this PR should fix #646 and you nicely introduced and documented a new variable for this feature.
However, that variable is not used anywhere in the code so it has no effect yet.
Before merging this PR, we also need to add this effect to make the story complete.
Otherwise after merge, we have a documentation promising a feature that is actually not there and we had stuff like that recently what confused our pilot users.
Please add a change that will redirect the output to a log-file according to the value of this new property.
IMHO this should not only happen for IntelliJ but for any tool we run as BACKGROUND
as you already implied by the documentation.
Maybe we might even think of a more precise name for the variable since IDE_ENABLE_CONSOLE_LOG
may imply that all console output is written to a logfile what is actually not the case.
PR will be closed for now, since it is still unclear how we will handle console logging from other IDEs, and intellij console log will be disabled by default. |
fixes #646
tied to: devonfw/ide-settings#66