Skip to content
This repository has been archived by the owner on Oct 9, 2024. It is now read-only.

Remove console.debug statements #541

Closed
wants to merge 2 commits into from

Conversation

sc68cal
Copy link

@sc68cal sc68cal commented Jan 10, 2023

This is related to ansible/vscode-ansible#1142

This is related to #540
@sc68cal sc68cal requested a review from a team as a code owner January 10, 2023 20:17
Copy link
Contributor

@priyamsahoo priyamsahoo left a comment

Choose a reason for hiding this comment

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

In place of console.log and console.debug, try using connection.console.log and connection.console.debug and see it's effects.

These logs are vital for us at the time of solving bugs, so complete removal of it would be a problem for us.

@ssbarnea
Copy link
Member

@sc68cal @priyamsahoo Yep, we need these logged, so we need an alternative that is safe to use that would not break the protocol. I wonder if could monkeypatch console logging to produce messaging that are not breaking the protocol, like JSON ones?

If is not possible to patch console logging to make it safe to use, we probably want to add a linter check for preventing accidental introduction in the future. I wonder if eslint can help us achieve that.

@sc68cal sc68cal closed this Apr 2, 2023
@sc68cal
Copy link
Author

sc68cal commented Apr 2, 2023

Attempted to bring this issue up, but I do not have time to spend on this, more than what I did in January.

JosBritton added a commit to JosBritton/ansible-language-server that referenced this pull request Oct 18, 2023
Do not log validation status on stdout, use connection logs instead.
See #540, ansible#541
priyamsahoo added a commit that referenced this pull request Oct 23, 2023
* Stdio patch
Do not log validation status on stdout, use connection logs instead.
See #540, #541

* Fix: use optional chaining for connection as it may not exist for purposes of testing.

---------

Co-authored-by: Priyam Sahoo <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants