Skip to content

Better cloning with NodeGit#453

Merged
tbranyen merged 4 commits into
masterfrom
better-clone
Mar 2, 2015
Merged

Better cloning with NodeGit#453
tbranyen merged 4 commits into
masterfrom
better-clone

Conversation

@tbranyen
Copy link
Copy Markdown
Member

@tbranyen tbranyen commented Mar 2, 2015

Right now we have the current call mechanism for clone:

var Git = require('nodegit');

Git.Clone.clone('https://github.com/nodegit/nodegit', 'nodegit').then(function(repo) {
  /* work with the repository */
});

This is pretty verbose, compared to:

var Git = require('nodegit');

Git.Clone('https://github.com/nodegit/nodegit', 'nodegit').then(function(repo) {
  /* work with the repository */
});

So I've fixed that in the JS-side. The top-level NodeGit.Clone Object is now a Function that behaves like an Object.

This is not a backwards-breaking change, so hopefully we're all in favor?

Comment thread test/tests/clone.js Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Damnit!

tbranyen added 4 commits March 2, 2015 17:48
I find it really annoying that we have to do `NodeGit.Clone.clone` just
to clone a repository.  It would be much nicer to get a similar effect
like in C, where you can run `git_clone`.  This copies over the effect
of an object, but actually using our `clone` patched function instead.

I've made this commit first to show it does not introduce regressions
with our current clone code.
@johnhaley81
Copy link
Copy Markdown
Collaborator

This flows much better than NodeGit.Clone.clone

👍

@tbranyen
Copy link
Copy Markdown
Member Author

tbranyen commented Mar 2, 2015

Cool, merging!

tbranyen added a commit that referenced this pull request Mar 2, 2015
@tbranyen tbranyen merged commit 22430ec into master Mar 2, 2015
@tbranyen tbranyen deleted the better-clone branch March 2, 2015 23:00
@johnhaley81 johnhaley81 added this to the 0.3.0 milestone Mar 3, 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.

2 participants