Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update to use OmniSharp 0.19.0 #1424

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

Merged
andyleejordan merged 32 commits into master from andschwa/omnisharp-update
Mar 26, 2021
Merged

Conversation

@andyleejordan
Copy link
Member

@andyleejordan andyleejordan commented Mar 12, 2021
edited
Loading

This updates our base dependency, OmniSharp, from 0.18.3 to 0.19.0, which included many API changes. The approach taken to reduce code was the replacement of interface-implementations with ABC derivations, providing a few things for free for each of the derived classes, and is more inline with the expectations of the library developers. There are a couple classes that implement multiple interfaces, and have been left TODO for a switch to an ABC implementation. Furthermore, the C# language specification needed to be updated because of OmniSharp's use of new language features (namely records), and I found that our xUnit package was misaligned.

Copy link
Member Author

Working on getting this put together. FYI @rjmholt and @david-driscoll

@andyleejordan andyleejordan force-pushed the andschwa/omnisharp-update branch 3 times, most recently from 48d2ceb to bedcc39 Compare March 13, 2021 02:54
Copy link
Member Author

The PSES tests don't pass, but it builds! The extension tests pass, I think.

...HandlerBase`
Except for `TelemetryEventParams` changes, saving for later.
Which we need for `record`s, and OmniSharp forces us to use.
@andyleejordan andyleejordan marked this pull request as ready for review March 25, 2021 21:27
Copy link
Member Author

This is ready @rjmholt!

@andyleejordan andyleejordan merged commit 1ffe6db into master Mar 26, 2021
@andyleejordan andyleejordan deleted the andschwa/omnisharp-update branch March 26, 2021 17:13
Copy link
Member Author

Supersedes dependabot's PRs: #1419, #1421, and #1422.

rjmholt reacted with thumbs up emoji

Name = "Main Thread"
})
// TODO: This is an empty container of threads...do we need to make a thread?
Threads = new Container<System.Threading.Thread>()
Copy link
Member Author

@andyleejordan andyleejordan Apr 19, 2021

Choose a reason for hiding this comment

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

I thought this was weird, but we already didn't really know what to do with it (see "What do I do with these?")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rjmholt rjmholt rjmholt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-Language Server Issue-Enhancement A feature request (enhancement).

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /