- Notifications
You must be signed in to change notification settings - Fork714
Storage account for Copilot language server#8408
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…nt instead of relying on its presence in C# Dev Kit.
package.json Outdated
| { | ||
| "id": "RoslynCopilotLanguageServer", | ||
| "description": "Language server for Roslyn Copilot integration (Windows)", | ||
| "url": "https://roslynomnisharp.blob.core.windows.net/releases/1.39.12/omnisharp-linux-musl-arm64-net6.0-1.39.12.zip", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This url will change
| "arm64" | ||
| ], | ||
| "installTestPath": "./.roslyncopilotlanguageserver/Microsoft.VisualStudio.Copilot.Roslyn.LanguageServer.dll", | ||
| "integrity": "9944EBD6EE06BD595BCADD3057CD9BEF4105C3A3952DAE03E54F3114E2E6661F" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
so will this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
know this is a draft, just wanted to make a couple small notes
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Thank you, feedback is welcome. This is change is mostly ready. I'm just trying to get the bits deployed, so I can update the url and the sha before its good to merge. |
| "description": "Language server for Roslyn Copilot integration (Windows)", | ||
| "url": "https://roslynomnisharp.blob.core.windows.net/releases/1.39.12/omnisharp-linux-musl-arm64-net6.0-1.39.12.zip", | ||
| "installPath": ".roslyncopilot", | ||
| "platforms": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
if there is a better way of specifying support for all platforms and architectures the extension supports, I'd love to put that in so this list is a little more maintainable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could introduce the concept of "neutral" which would be a catch-all. Are there no plans to generate R2R builds of these libraries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Perhaps, in the future?
On the "neutral" piece, is that something we have the liberty to configure? I thought this was a fixed schema of some sort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It is used by thePackageManager functionality which builds out of our repo. In particular we would updateour package filter to also include these "neutral" packages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Cool, gotcha. As long as that's the only place that needs a change, happy to make that update. Thought this was consumed by other tooling. Thanks for the confirmation 👍
Uh oh!
There was an error while loading.Please reload this page.
This is primarily an implementation of David Barbet's suggestion that helps with our dev inner-loop reducing the amount of dependencies across repos. With the roslyn copilot language server coming in from the storage account, changes are no longer required in C# Dev Kit to point to the right language server.
Todo
Validation
Ensured that the context provider is indeed invoked after my changes. It pulls in the binary from the new location. So this was a debug setup on a windows devbox. I do not think other OS's could cause a problem but I'll figure out what it would take to validate this on mac and linux as well.