-
-
Notifications
You must be signed in to change notification settings - Fork 369
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
Do not send Heap Stats to the LSP log #3111
Conversation
It interferes with the ability of lsp-test to detect timeouts in e.g. benchmarks The assumption is that logging them to stderr serves the purpose they were designed for.
Would it make sense to just disable the client logs when doing tests or benchmarks? |
It's easy to do and I don't have a strong opinion |
9226cb8
to
43586ab
Compare
43586ab
to
eb32021
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.
Would it make sense to just disable the client logs when doing tests or benchmarks?
It's easy to do and I don't have a strong opinion
Then it looks good to me. Some users might be using this log as a quick measure to check memory consumption.
This PR doesn't remove the log from stderr, only from the LSP stream. We only started emitting those around 3 months ago so I doubt any users depend on them. |
I just think it makes our logging less consistent, which is confusing in future. But 🤷 |
It interferes with the ability of lsp-test to detect timeouts in e.g. benchmarks
The assumption is that logging them to stderr serves the purpose they
were designed for.