-
Notifications
You must be signed in to change notification settings - Fork 420
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
Improved the logic of adding default assembly references for Desktop CLR CSX scripts #898
Conversation
commonReferences.Add(reference); | ||
} | ||
|
||
Assembly FromName(string assemblyName) |
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.
Why not use an IAssemblyLoader
?
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.
and here I was, so happy that I used a local function... 😋
you are right though
I also noticed it needs a rebase
fae8f60
to
cd7b3c3
Compare
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.
Is there any point of adding any unit tests at all?
Good point. I think we should be able to have a test based on using the |
Actually the test infrastructure has it's own logic for default references, that's why this issue never showed up in tests before I think this should be good to merge for now then. |
Sounds good to me. @DustinCampbell ? |
Fine by me! |
everything is green. I think this is good to merge - no reason to keep it hanging around anymore |
This PR improves the way we are adding default assembly references for Desktop CLR CSX scripts. We need to make sure we add references to all assemblies type forwarders might point to when working with a Desktop CLR script (for .NET Core scripts, all references should be provided via the project model).
The main difference was so far we references
mscorlib.dll
andSystem.Core.dll
but missedSystem.dll
.Fixes dotnet/vscode-csharp#1581