Skip to content

Conversation

tgodzik
Copy link
Collaborator

@tgodzik tgodzik commented Mar 27, 2024

No description provided.

tgodzik and others added 30 commits March 27, 2024 10:31
Previously, we would use the default config, which would not always be correct. Now, we use runtime config with a fallback to normal platform config.
Previously, this would be set to error, which would mean Metals would show it to the users more prominently. Now, we change it to warn, which means it's still present, but since this might be actually be related to non compiled code, it's prominence is reduced,
As far as I can see this should work properly and it hits us even more to have duplicated part of code that is not maintained.
It seems we don't get that properly forwarded to Bloop otherwise.
kpodsiad and others added 21 commits March 27, 2024 14:33
This might be a bit opinionated and I'm not 100% sure on the reason this
was done in the first place, but this changes the current behavior
around reporting start and end compilations for no-ops. This still
retains the compile report if it was a no-op and also still checks the
diagnostics and reports them, but skips the task start/end.

Originally this was added in a commit without any context... but there
was a comment that said:

> // When no-op, we keep reporting the start and the end of compilation for consistency

However, I'm not really sure that consistency matters here. In reality
this ends up creating a bunch of noise on the client side, especially
when these tasks turn into LSP progress notifications that aren't useful
for the user to see. You can see more context about this change in the
issue reported [here](scalameta/metals#6099)
and also the discussion found
[here](build-server-protocol/build-server-protocol#654).
It also seems that in some projects like scala-cli these are even [ignored](https://github.com/VirtusLab/scala-cli/blob/6b7a10007e4eefde717079255e0df38c027f788b/modules/build/src/main/scala/scala/build/ConsoleBloopBuildClient.scala#L109).
@tgodzik tgodzik merged commit 967aa1b into main Mar 27, 2024
@tgodzik tgodzik deleted the update branch March 27, 2024 19:11
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.

7 participants