remote: add support for ls-remote #609
Conversation
remote.go
Outdated
| @@ -728,6 +728,39 @@ func (r *Remote) buildFetchedTags(refs memory.ReferenceStorage) (updated bool, e | |||
| return | |||
| } | |||
|
|
|||
| // LSRemote performs ls-remote on the remote. | |||
| func (r *Remote) LSRemote(auth transport.AuthMethod) ([]*plumbing.Reference, error) { | |||
There was a problem hiding this comment.
Maybe we should call this method List?
// List the references on the remote repository
Thoughts @mcuadros?
There was a problem hiding this comment.
Also, does it make sense to generalize the parameters so that it accepts an Options-style struct instead of just transport.AuthMethod to help ease future changes without breaking API compatibility?
There was a problem hiding this comment.
Yep, a Options will be the best
There was a problem hiding this comment.
Yep, makes sense. I created an Option at first, but then dropped it thinking it would be simpler to just pass the AuthMethod directly 😅. Adding it back...
Codecov Report
@@ Coverage Diff @@
## master #609 +/- ##
=========================================
- Coverage 78.32% 77.7% -0.62%
=========================================
Files 130 130
Lines 10140 10160 +20
=========================================
- Hits 7942 7895 -47
- Misses 1347 1423 +76
+ Partials 851 842 -9
Continue to review full report at Codecov.
|
remote_test.go
Outdated
| }) | ||
|
|
||
| rs := config.RefSpec("refs/heads/*:refs/heads/*") | ||
| err = remote.Push(&PushOptions{ |
There was a problem hiding this comment.
Since Push and List will basically use the same code under the hood, we are sort of asserting this code's behavior against itself.
Maybe it makes for sense to explicitly define the expected references for this fixture repository? Also this gives us a chance to assert that symbolic references are handled properly rather than ignoring them.
func (s *RemoteSuite) TestList(c *C) {
repo := fixtures.Basic().One()
remote := newRemote(memory.NewStorage(), &config.RemoteConfig{
Name: DefaultRemoteName,
URLs: []string{repo.URL},
})
refs, err := remote.List(&ListOptions{})
c.Assert(err, IsNil)
expected := []*plumbing.Reference{
plumbing.NewSymbolicReference("HEAD", "refs/heads/master"),
plumbing.NewReferenceFromStrings("refs/heads/master", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5"),
plumbing.NewReferenceFromStrings("refs/heads/branch", "e8d3ffab552895c19b9fcf7aa264d277cde33881"),
plumbing.NewReferenceFromStrings("refs/pull/1/head", "b8e471f58bcbca63b07bda20e428190409c2db47"),
plumbing.NewReferenceFromStrings("refs/pull/2/head", "9632f02833b2f9613afb5e75682132b0b22e4a31"),
plumbing.NewReferenceFromStrings("refs/pull/2/merge", "c37f58a130ca555e42ff96a071cb9ccb3f437504"),
}
c.Assert(len(refs), Equals, len(expected))
for _, e := range expected {
found := false
for _, r := range refs {
if r.Name() == e.Name() {
found = true
c.Assert(r, DeepEquals, e)
}
}
c.Assert(found, Equals, true)
}
}
There was a problem hiding this comment.
Thanks. Makes sense. 🙂
Adds support for ls-remote in
Remote.Found this in #601-comment, so maybe fixes #601 .
Need some guidance in the test with relevant existing examples. Not sure if this test is enough.