Clean up unresolved dependencies check #862
Conversation
|
|
||
| namespace OmniSharp.MSBuild.Resolution | ||
| { | ||
| internal class PackageDependencyResolver |
There was a problem hiding this comment.
Is there much value in writing tests for this class?
There was a problem hiding this comment.
I think there's definitely value in having tests, but I don't want to gate this PR on them. I want to write tests for the MSBuildProjectSystem in general because there's really not much today. Now that we have the test infrastructure in place, I'd like to write some broader MSBuildProjectSystem tests.
There was a problem hiding this comment.
Added #863 to track adding tests.
| var unresolved = libraryManager.GetLibraries().Where(dep => !dep.Resolved); | ||
| var needRestore = allDiagnostics.Any(diag => diag.ErrorCode == ErrorCodes.NU1006) || unresolved.Any(); | ||
| var unresolvedLibraries = libraryManager.GetLibraries().Where(dep => !dep.Resolved); | ||
| var needRestore = allDiagnostics.Any(diag => diag.ErrorCode == ErrorCodes.NU1006) || unresolvedLibraries.Any(); |
There was a problem hiding this comment.
when lock file doesn't exist at all I believe the error code would be NU1009.
Should that be taken into account as well? a restore would then generate the lock file
There was a problem hiding this comment.
actually, maybe it doesn't matter anymore - it's legacy
There was a problem hiding this comment.
Yeah, I wasn't trying to clean up DotNetProjectSystem too much. I've been thinking of that as a bit frozen so I don't unintentionally break anything.
Fixes dotnet/vscode-csharp#1272
This change cleans up the logic (which had gotten kinda messy) that resolves MSBuild package references against a project's lock file. This also adds logging when package references can't be resolved, which should help diagnose situations where VS Code is getting restore prompts unnecessarily.