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

use release omnisharp #188

Merged
merged 1 commit into from
Aug 13, 2015
Merged

use release omnisharp #188

merged 1 commit into from
Aug 13, 2015

Conversation

corngood
Copy link

@corngood corngood commented Aug 4, 2015

Using the release configuration of OmniSharp.

*** There are currently bugs with the Release configuration that need to be fixed with OmniSharp/omnisharp-server#206, so this shouldn't be merged until the OmniSharp fix is merged and the submodule updated. I'll update this PR if/when that happens.

@corngood
Copy link
Author

This change now contains the submodule update to include the fix in omnisharp-server.

@vheon
Copy link
Contributor

vheon commented Aug 12, 2015

What I didn't understand is what this should bring to the user and at this point why we were using the debug version of Omnisharp.

@Valloric
Copy link
Member

Thanks!

@homu r+

@vheon AFAIR for .Net projects the distinction between debug/release isn't really significant. There isn't a large perf difference as with C++ projects. More info.

@homu
Copy link
Contributor

homu commented Aug 13, 2015

📌 Commit 94c1d93 has been approved by Valloric

homu added a commit that referenced this pull request Aug 13, 2015
WIP: use release omnisharp

Using the release configuration of OmniSharp.

*** There are currently bugs with the Release configuration that need to be fixed with OmniSharp/omnisharp-server#206, so this shouldn't be merged until the OmniSharp fix is merged and the submodule updated.  I'll update this PR if/when that happens.
@homu
Copy link
Contributor

homu commented Aug 13, 2015

⌛ Testing commit 94c1d93 with merge 3ea9664...

@corngood corngood changed the title WIP: use release omnisharp use release omnisharp Aug 13, 2015
@corngood
Copy link
Author

I'm not sure why it was originally using Debug, but it could be because of the solution configuration problem I referenced above. Release in the omnisharp .sln was building the silverlight configuration of some dependencies, which stopped it from locating mscorlib etc.

As far as performance goes, I unfortunately don't have hard numbers at the moment, but I did this originally because I was trying to profile a severe performance problem in one of the code issue modules (icsharpcode/NRefactory#476). This sort of thing is CPU limited managed code, so there is a decent improvement.

@homu
Copy link
Contributor

homu commented Aug 13, 2015

☀️ Test successful - status

@homu homu merged commit 94c1d93 into ycm-core:master Aug 13, 2015
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.

5 participants