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

Direct repo #29

Merged
merged 7 commits into from Feb 4, 2020
Merged

Direct repo #29

merged 7 commits into from Feb 4, 2020

Conversation

futuraprime
Copy link
Contributor

@futuraprime futuraprime commented Mar 6, 2018

This is meant to address #19, though it may also address some other requests like #20 and #21 (I haven't tested them).

It gives degit another route to achieving its end, by simply cloning the repository and then eliminating all the git data within. It's neither pretty nor efficient, but it does work, even on private repositories. Degit automatically falls back on it if the repository given doesn't match degit's supported handlers (which would previously have been an error).

It might be nice to include further error checking to ensure it's received a valid git SSH or HTTPS endpoint. Without that it will simply fail to clone the repo and return an error at that point. I'm not sure whether it's wise to try to duplicate git's URI validation.

@futuraprime
Copy link
Contributor Author

Irritatingly, it looks like the git library I've used (https://github.com/nodegit/nodegit) doesn't play nicely with CI. I'm not sure what would be a good way round that.

@futuraprime
Copy link
Contributor Author

I've sorted out the problem with Travis (by reading NodeGit's readme, amazing), and the tests are passing now (they need an extremely long timeout).

@Rich-Harris
Copy link
Owner

Thank you! Apologies for not getting to this much sooner.

I'm hesitant to add nodegit to this repo, as it will substantially increase installation times. Since degit is often used via npx, we need to keep installation as speedy as possible. So in the process of resolving merge conflicts, I changed the implementation to just shell out to git, which hopefully works for most people.

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

2 participants