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.

Sign up for GitHub

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

Jump to bottom

Re-publish all warnings on BSP server startup #729

Merged
merged 2 commits into from
Nov 21, 2018
Merged

Re-publish all warnings on BSP server startup #729

merged 2 commits into from
Nov 21, 2018

Conversation

jvican
Copy link
Contributor

@jvican jvican commented Nov 21, 2018 •
edited
Loading

... and use the correct file URI schema for the files in the diagnostics notifications. More info in the commit messages.

@jvican
Re-publish warnings on BSP server startup
8aaf828 
Fixes #726
@jvican
Use file:/// instead of file:/ for BSP URIs
64c47b8 
Avoid the use of `(f: File).toURI` and prefer `(p: Path).toUri` which
produces a correct URI scheme.

Fixes #728
@jvican jvican added build server Any issue or pull request that has to do with hot compilers or BSP. bug A defect or misbehaviour. enhancement labels Nov 21, 2018
@jvican jvican requested a review from olafurpg November 21, 2018 09:12
olafurpg
olafurpg approved these changes Nov 21, 2018
View reviewed changes
Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I am a bit concerned by the growing test case in BspProtocolSpec. Down the road, I would look for ways to make it easier to write new small self-contained unit tests over extending existing large test cases.

frontend/src/main/scala/bloop/logging/BspServerLogger.scala Show resolved Hide resolved
frontend/src/test/scala/bloop/bsp/BspProtocolSpec.scala Show resolved Hide resolved
@jvican
Copy link
Contributor Author

jvican commented Nov 21, 2018

I am a bit concerned by the growing test case in BspProtocolSpec

Agreed, I would like to take some time down the road to deduplicate some of the logic here.

@jvican jvican merged commit b59cd8a into master Nov 21, 2018
tindzk pushed a commit to tindzk/bloop that referenced this pull request Nov 21, 2018
@jvican @tindzk
Merge pull request scalacenter#729 from scalacenter/ticket/726
a433b8d 
Re-publish all warnings on BSP server startup
@tgodzik tgodzik deleted the ticket/726 branch September 7, 2021 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olafurpg olafurpg olafurpg approved these changes

Assignees
No one assigned
Labels
bug A defect or misbehaviour. build server Any issue or pull request that has to do with hot compilers or BSP. enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jvican @olafurpg