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

Allow URLs as source locations #294

Merged

Conversation

jvasiljevich
Copy link

This PR was made to implement enhancement described in #293

@westeezy
Copy link

👍 This change would be very helpful to our project.

@jvasiljevich jvasiljevich force-pushed the accept-url-sources branch 2 times, most recently from 79717a6 to 2225a72 Compare February 25, 2015 23:19
@madorb
Copy link

madorb commented Feb 26, 2015

This change would be very helpful to MY project

@joelittlejohn
Copy link
Owner

Thanks for working on this. There's quite a few changes so I'd quite like to give this a thorough look through before merging, but in general I don't see a problem with adding this feature in the way you have.

I noticed that the integration tests are currently failing, could you take a look at this?

@joelittlejohn joelittlejohn added this to the 0.4.9 milestone Feb 26, 2015
@jvasiljevich
Copy link
Author

@joelittlejohn thanks for taking a look! Regarding the test failure, it looks like it's only a single test: specifically, the pathWithSpacesInTheNameDoesNotFail test within RegressionIT.

Oddly enough, when run from my IDE, that test also fails on master. It does pass on master when run from the command line though...seems strange. At any rate, I'll take a closer look at it.

@joelittlejohn
Copy link
Owner

I expect this is something to do with basedir when running in your IDE.

On 26 February 2015 at 22:47, jvasiljevich notifications@github.com wrote:

@joelittlejohn https://github.com/joelittlejohn thanks for taking a
look! Regarding the test failure, it looks like it's only a single test.
Specifically, the pathWithSpacesInTheNameDoesNotFail test that's failing
within RegressionIT.

Oddly enough, when run from my IDE, that test also fails on master. It
does pass on master when run from the command line though...seems strange.
At any rate, I'll take a closer look at it.


Reply to this email directly or view it on GitHub
#294 (comment)
.

@jvasiljevich
Copy link
Author

Had a chance to look at the failing test this morning. Fixed a url decoding issue in the node naming functionality. Tests are passing now.

joelittlejohn added a commit that referenced this pull request Mar 5, 2015
@joelittlejohn joelittlejohn merged commit 7f0eaee into joelittlejohn:master Mar 5, 2015
@joelittlejohn joelittlejohn changed the title Accept url sources Allow URLs as source directories Mar 6, 2015
@joelittlejohn joelittlejohn changed the title Allow URLs as source directories Allow URLs as source locations Mar 6, 2015
@joelittlejohn joelittlejohn removed this from the 0.4.9 milestone Mar 6, 2015
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.

None yet

4 participants