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 NuGetPackageResolver in InteractiveHost #5356

Merged
merged 3 commits into from
Sep 23, 2015
Merged

Use NuGetPackageResolver in InteractiveHost #5356

merged 3 commits into from
Sep 23, 2015

Conversation

cston
Copy link
Member

@cston cston commented Sep 19, 2015

  • Use resolver in InteractiveHost.Service
  • Change reference syntax to #r "nuget:name/version"
  • Use local cache only for resolving packages

@cston
Copy link
Member Author

cston commented Sep 19, 2015

@@ -46,24 +39,26 @@ internal sealed class RuntimeMetadataReferenceResolver : MetadataReferenceResolv

public override ImmutableArray<PortableExecutableReference> ResolveReference(string reference, string baseFilePath, MetadataReferenceProperties properties)
{
if (PathResolver != null && PathUtilities.IsFilePath(reference))
if (PackageResolver != null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be clearer to parse up front and then try either nuget resolution or file resolution. Consider adding a TryParse method that returns a refined string to be passed to ResolveNuGetPackage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@mmitche
Copy link
Member

mmitche commented Sep 22, 2015

Can one of the admins approve this PR test or whitelist this user? ('@dotnet-bot add to whitelist' to whitelist, '@dotnet-bot okay to test' to accept for PR, '@dotnet-bot test this please' to retest. Case sensitive).

@cston
Copy link
Member Author

cston commented Sep 23, 2015

@dotnet-bot retest this please

@khellang
Copy link
Member

Just curious; Why is the package version required?

@cston
Copy link
Member Author

cston commented Sep 23, 2015

@khellang The package version should be optional. We'll remove the version requirement in a separate PR.

@khellang
Copy link
Member

Agreed. Awesome 👍

cston added a commit that referenced this pull request Sep 23, 2015
Use NuGetPackageResolver in InteractiveHost

- Use resolver in InteractiveHost.Service
- Change reference syntax to #r "nuget:name/version"
- Use local cache only for resolving packages
@cston cston merged commit a90e0e6 into dotnet:master Sep 23, 2015
@cston cston deleted the ng-2 branch September 23, 2015 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants