Conversation
ehuss
left a comment
ehuss
left a comment
There was a problem hiding this comment.
Thanks for adding this!
Can you also include a test for it?
| /// Specify fetch depth, a value less than zero is interpreted as pull everything (effectively the same as no declaring a limit depth) | ||
| pub fn depth(&mut self, depth: i32) -> &mut RepoBuilder<'cb> { |
There was a problem hiding this comment.
Instead of adding a new method here, would it be possible to leave this as a setting that required calling .fetch_options(...) instead? I am reluctant to try to repeat low-level option settings, since that would require deciding which settings are or are not replicated, and possibly causing confusion when something could be specified in multiple places.
There was a problem hiding this comment.
So should this be removed? I find the depth parameter to be kinda (very) important imo, but it can be removed without any problems.
| // if let Some(ref fetch_opts) = &self.fetch_opts { | ||
| // fetch_opts.depth(depth); | ||
| // } else { | ||
| // let mut fo = FetchOptions::new(); | ||
| // fo.depth(depth); | ||
| // self.fetch_options(fo); | ||
| // } | ||
| // self |
There was a problem hiding this comment.
Looks to be some vestigial work?
| /// Clone a remote repository with a depth of one (shallow cloning) | ||
| /// | ||
| /// Similar to `git clone --depth 1` | ||
| pub fn shallow_clone<P: AsRef<Path>>(url: &str, into: P) -> Result<Repository, Error> { |
There was a problem hiding this comment.
Similar to the comment on RepoBuilder, I think I would like to avoid having too many convenience functions, since there is a somewhat combinatorial explosion of all the different options someone might want. Using a builder pattern can avoid adding all sorts of variants of different methods.
|
I'm not sure how to add a test for this, it seems like there aren't any tests for cloning features. |
At the bottom of |
|
This new commit addresses the comments and contains a first attempt to test it. It should work but it doesn't, I need some guidance here. The test creates a repo and commits two times to that new repo. Then creates a |
|
Any work on this? |
|
Unfortunately it looks like libgit2 does not do shallow clones on local repositories. I think it requires the smart protocol and relies on the server to figure out what to fetch. I don't think we have any testing infrastructure for the smart protocol, so I think we'll just need to not include a test for this for now. I have filed libgit2/libgit2#6634 on that issue. |
|
So for now, it can go without a test? (I'll add a fixme) |
|
Yep. |
blyxyas
force-pushed
the
shallow-clone
branch
2 times, most recently
from
4680cd1 to
722b7d0
Compare
|
Why it has not been merged yet? I mean is there nay issue preventing it? |
ehuss
force-pushed
the
shallow-clone
branch
from
a901c4c to
1441a8d
Compare
ehuss
force-pushed
the
shallow-clone
branch
from
1441a8d to
7a80cc2
Compare
3 participants
Add shallow cloning (as well as customizable depth), introduced in
libgit2 v1.7.0