Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

Update the priority of finding matchs among walk providers #1000

Merged
merged 1 commit into from
Dec 19, 2014

Conversation

troydai
Copy link
Contributor

@troydai troydai commented Dec 18, 2014

Break the walk providers into to group. The groups use a local file system
based source take the priority.

#999

@troydai
Copy link
Contributor Author

troydai commented Dec 18, 2014

@loudej @davidfowl

For following case:

kpm restore -s C:\localsource\ -s https://www.myget.org/F/aspnetvnext/api/v2

The kpm won't hit http if all the packages are found under c:\localsource\

@@ -28,6 +25,7 @@ public interface IWalkProvider
Task<WalkProviderMatch> FindLibraryBySnapshot(Library library, FrameworkName targetFramework);
Task<IEnumerable<LibraryDependency>> GetDependencies(WalkProviderMatch match, FrameworkName targetFramework);
Task CopyToAsync(WalkProviderMatch match, Stream stream);
bool Express { get; }
Copy link
Member

Choose a reason for hiding this comment

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

What?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Help me find a better mean? Essentially it means this provider is faster so should be try first.

Copy link
Member

Choose a reason for hiding this comment

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

Express is a bad name. Order?

Copy link
Member

Choose a reason for hiding this comment

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

IsLocal? Say what it is, not how someone will consume it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, maybe IsLocal or IsHttp (and do the negation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let's go with IsLocal.

@davidfowl
Copy link
Member

Fix the typo in your commit message

@troydai
Copy link
Contributor Author

troydai commented Dec 18, 2014

Yup.

@troydai
Copy link
Contributor Author

troydai commented Dec 18, 2014

@loudej

@davidfowl
Copy link
Member

Lets do IsHttp the term local is overused

@troydai
Copy link
Contributor Author

troydai commented Dec 18, 2014

aye aye, sir.

@troydai
Copy link
Contributor Author

troydai commented Dec 18, 2014

Rebased.

@troydai
Copy link
Contributor Author

troydai commented Dec 18, 2014

@PradeepKadubandi

@@ -37,8 +35,11 @@ public class LocalWalkProvider : IWalkProvider
public LocalWalkProvider(IDependencyProvider dependencyProvider)
{
_dependencyProvider = dependencyProvider;
IsHttp = false;
Copy link
Member

Choose a reason for hiding this comment

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

Don't need this, it's the default value.

@davidfowl
Copy link
Member

Ship it

Break the remote sources into two groups. The group of the http based
sources won't be executed unless the group of the local sources can't produce
a match. Among the same group all the sources are still executed parallel.
@troydai
Copy link
Contributor Author

troydai commented Dec 19, 2014

Rebased and squashed.

@troydai troydai merged commit 17de54d into dev Dec 19, 2014
@troydai troydai deleted the update.kpmrestore branch December 19, 2014 18:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants