Update Versions.props and remove loghub references#51966
Update Versions.props and remove loghub references#51966dibarbet merged 4 commits intodotnet:main-vs-depsfrom
Conversation
| <MicrosoftCodeAnalysisTestingVersion>1.0.1-beta1.20623.3</MicrosoftCodeAnalysisTestingVersion> | ||
| <CodeStyleAnalyzerVersion>3.8.0</CodeStyleAnalyzerVersion> | ||
| <VisualStudioEditorPackagesVersion>16.9.220</VisualStudioEditorPackagesVersion> | ||
| <VisualStudioEditorPackagesVersion>16.10.44</VisualStudioEditorPackagesVersion> |
There was a problem hiding this comment.
Does this need to merge into main-vs-deps?
There was a problem hiding this comment.
Is there any risk to merge this to main-vs-deps?
There was a problem hiding this comment.
roslyn needs to be runnable on 16.9. So if these introduce 16.10 dependencies that are not present in 16.9 and fail to load, then this needs to go into the -vs-deps branch. I presume since these are editor dependencies 16.9 will fall on the floor
kelltrick
left a comment
There was a problem hiding this comment.
Looks good, I left a single comment which is to take advantage of a new parameter to automatically do what you were doing explicitly.
src/VisualStudio/Core/Def/Implementation/LanguageClient/InProcLanguageServer.cs
Outdated
Show resolved
Hide resolved
|
Updating to try and to get this in before the loghub dependency is removed. |
| <ILDAsmPackageVersion>5.0.0-preview.1.20112.8</ILDAsmPackageVersion> | ||
| <MicrosoftVisualStudioLanguageServerProtocolPackagesVersion>16.9.150</MicrosoftVisualStudioLanguageServerProtocolPackagesVersion> | ||
| <MicrosoftVisualStudioShellPackagesVersion>16.9.30921.310</MicrosoftVisualStudioShellPackagesVersion> | ||
| <MicrosoftVisualStudioShellPackagesVersion>16.10.0-preview-2-31112-292</MicrosoftVisualStudioShellPackagesVersion> |
There was a problem hiding this comment.
It will likely be a few weeks before a 16.10 preview2 build is available on the integration test queue. Should we start a validation insertion for this PR?
There was a problem hiding this comment.
@JoeRobich before I updated the feedback and merged in main-vs-deps, the integration tests were actually passing! But I will trigger a validation anyway due to all the upgrades.
Only the LSP ones were failing, but I tested in preview1 (which is being rolled out to scouting tomorrow) and that appeared to fix them.
There was a problem hiding this comment.
@JoeRobich validation seems fine, jsut hit the same ngen issues main is facing - https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/312719
There was a problem hiding this comment.
I plan to merge even with the LSP tests failing as they would be fixed on the queue update to 16.10p1 (and only failing in main-vs-deps).
Update a few versions, and remove the loghub references.
Platform team is moving loghub to VisualStudio.Utilities.dll. So log hub should be removed.
If not removed, when updates the versions, there would be errors complaining about two same classes found in different assemblies.
I encountered this error when I am updating some dlls versions and try to consume a new Image moniker. But I feel it is meaningful to first port the change to main.