Make Pull and Fetch commands#1288
Conversation
|
Maybe put them in extension methods of Network? The class approach would kinda work if you would make the arguments accessible: private readonly Repository repository;
private readonly Signature merger;
private readonly PullOptions options; |
| using (var repo = new Repository(path)) | ||
| { | ||
| Remote remote = repo.Network.Remotes.Add(remoteName, url); | ||
| repo.Network.Remotes.Add(remoteName, url); |
There was a problem hiding this comment.
Wouldn't this cause an issue, since Remote is now IDisposable?
I know this is a test method, but we should show the _right thing_™
There was a problem hiding this comment.
It's in the same situation as before. We dispose of the remotes on repository disposal.
|
Making this extension methods is the opposite direction to what this is trying to achieve. It's an attempt at making clear what is an operation and what is a method that's trying to replicate what you'd call on the command line. |
This makes it clearer that what we have there is a command and not a method fundamental to the Git system.
6af5806 to
7520d62
Compare
7520d62 to
53dc1c2
Compare
Since we make no use of these commands being classes. Make them static methods of a static class to give them some namespacing.
|
Yep. I like this.
Nah. Overkill, I think. |
|
LET THE COMMANDIFICATION BEGIN. |
These want to behave like commands, so let's put them in a namespace that says as much.
This is similar to #1248 but I'm touching the network area so I wanted to send this one as well, so we can discuss the design with more data points.
In particular, should these be classes? They don't really do anything that needs a class. A static method in a static class might work out better for keeping namespacing but not forcing undue instantiations.
These commands take a string instead of having
Remoteandstringvariants since that's what a command would take and all theRemoteinstances were doing was carry the name.