Skip to content

Network.Fetch() accepts optional refspec#585

Merged
nulltoken merged 2 commits intolibgit2:vNextfrom
Yogu:fetch-refspecs
Dec 12, 2013
Merged

Network.Fetch() accepts optional refspec#585
nulltoken merged 2 commits intolibgit2:vNextfrom
Yogu:fetch-refspecs

Conversation

@Yogu
Copy link
Contributor

@Yogu Yogu commented Dec 10, 2013

Added RefSpecs property to FetchOptions to allow for custom refspecs to
be fetched. The refspecs are used only temporarily and not stored in the
configuration.

Fixes #572. Depends on #584.

Maybe we should make refSpecs an argument for Fetch instead of a property of FetchOptions? It changes the semantics of the fetch operation significantly.

Yogu
Yogu reviewed Dec 10, 2013
View reviewed changes
LibGit2Sharp.Tests/FetchFixture.cs Outdated
Copy link
Contributor Author

@Yogu Yogu Dec 10, 2013

Choose a reason for hiding this comment

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

If TagFetchMode.None is not specified, tags are fetched. I'm not happy about this. Maybe Fetch should use None if TagFetchMode is set to null and RefSpecs is specified?

Copy link
Member

@carlosmn carlosmn Dec 10, 2013

Choose a reason for hiding this comment

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

Not quite. Tags refs are only created locally if you get tags for stuff you're downloading anyway. These are the git-fetch semantics, and it's what you should expect by default.

Copy link
Member

@carlosmn carlosmn Dec 10, 2013

Choose a reason for hiding this comment

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

Note that the previous comment applies as long as the user hasn't overriden the tag auto-following option in their config.

@Yogu Yogu mentioned this pull request Dec 12, 2013
Merged
Yogu added 2 commits December 12, 2013 21:39
@Yogu
Refactored Network.Fetch() with FetchOptions
dea2d7b 
Changed the signature to Network.Fetch(Remote, FetchOptions). As of
libgit2#536, optional parameters should be grouped in a xxxOptions type.
Network.Fetch() has currently 5 optional parameters, and a sixth one
(RefSpecs) will be introduced soon.
@Yogu
Network.Fetch() accepts optional refspec
27cd2dc 
Added Network.Fetch(Remote, IEnumerable<string>, [FetchOptions]) overload to allow for custom refspecs to
be fetched. The refspecs are used only temporarily and not stored in the configuration.

Fixes libgit2#572.
@Yogu
Copy link
Contributor Author

Yogu commented Dec 12, 2013

Regarding the memory-remote overload introduced in 1374884, I refactored the RefSpecs property to a parameter of a new Fetch overload. I think this makes the three usages of Fetch more clear:

  • Fetch a remote as configured
  • Fetch a remote, but with custom refspecs
  • Fetch from url (with custom refspecs)

@nulltoken nulltoken merged commit 27cd2dc into libgit2:vNext Dec 12, 2013
@nulltoken
Copy link
Member

nulltoken commented Dec 12, 2013

Brilliant!

@Yogu Yogu deleted the fetch-refspecs branch December 13, 2013 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v0.15.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Yogu @nulltoken @dahlbyk @carlosmn