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

improve dependency fetcher with clearer parameters #279

Merged
merged 6 commits into from
Feb 23, 2017
Merged

Conversation

mmcky
Copy link
Contributor

@mmcky mmcky commented Jan 11, 2017

@oyamad Once this is merged -- unfortunately this will break the notebook. But I think you guys are right -- this will be clearer to use.

@cc7768 Is this what you mean re issue #278?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.503% when pulling 4a358bc on update-nb-fetcher into 75e12dc on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.503% when pulling b4dc277 on update-nb-fetcher into 75e12dc on master.

@cc7768
Copy link
Member

cc7768 commented Jan 12, 2017

Yes, this is roughly what I had in mind. I think the dictionary behavior for files was a bit confusing because the folder name could be specified in two different places (either as the key in the dictionary or as the deps argument).

It would be nice if we could have someone write a short example of fetching files from each of the different repositories and include it in the documentation for this function to make it easier for others to put this function to work.

@mmcky
Copy link
Contributor Author

mmcky commented Feb 22, 2017

Before I merge this I really should write a test.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 82.857% when pulling c5c90c1 on update-nb-fetcher into 75e12dc on master.

@mmcky mmcky merged commit 91eaf1f into master Feb 23, 2017
@mmcky mmcky deleted the update-nb-fetcher branch February 23, 2017 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants