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

Bsp errors in status #5726

Merged
merged 8 commits into from
Nov 21, 2023
Merged

Conversation

kasiaMarek
Copy link
Contributor

@kasiaMarek kasiaMarek commented Oct 6, 2023

bloop-error

resolves: #5692

@kasiaMarek kasiaMarek force-pushed the bsp-errors-in-status branch from 3373773 to 04938f5 Compare October 13, 2023 15:26
@kasiaMarek kasiaMarek changed the title Bsp errors in status [WIP] Bsp errors in status Oct 13, 2023
@kasiaMarek kasiaMarek marked this pull request as ready for review October 13, 2023 15:27
@kasiaMarek kasiaMarek marked this pull request as draft October 19, 2023 12:33
@kasiaMarek kasiaMarek force-pushed the bsp-errors-in-status branch from 04938f5 to 95dd81a Compare October 25, 2023 15:00
@kasiaMarek kasiaMarek force-pushed the bsp-errors-in-status branch from c080bef to bc92724 Compare October 26, 2023 14:04
@kasiaMarek kasiaMarek marked this pull request as ready for review October 26, 2023 14:05
Copy link
Member

@jkciesluk jkciesluk left a comment

Choose a reason for hiding this comment

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

Great work! Updating state in ConnectionBspStatus could use some docs, but mostly looks good.

@kasiaMarek kasiaMarek force-pushed the bsp-errors-in-status branch from b2002b7 to c406ddc Compare October 27, 2023 13:48
Copy link
Member

@jkciesluk jkciesluk left a comment

Choose a reason for hiding this comment

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

LGTM!

"warn",
show = true,
tooltip = message.trimTo(TOOLTIP_MAX_LENGTH),
command = ServerCommands.RunDoctor.id,
Copy link
Member

Choose a reason for hiding this comment

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

As a follow up, would it be possible (probably on vs-code side) to make doctor open on the part with error reports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sounds like a good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also open them in that case, currently they are folded.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Looks great, I mostly have some questions to make sure I understand everything.

@kasiaMarek kasiaMarek requested a review from tgodzik November 20, 2023 10:33
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM!

@kasiaMarek kasiaMarek merged commit 57eb284 into scalameta:main Nov 21, 2023
24 checks passed
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.

Save bsp errors in reports and show in metals doctor
3 participants