Provide a convenience function git_remote_push()
#2608
Conversation
|
👍 |
include/git2/remote.h
Outdated
There was a problem hiding this comment.
Is refspecs optional here? Because I'd expect to be able to pass no refspecs here and use the default push specs as set in the config for a remote. That's how it works in the current Remote#push implementation. I know that by default, no push spec is set up, but I'd still like this to mirror the behavior of git_remote_fetch.
There was a problem hiding this comment.
It's not optional, but if you pass in an empty one, then nothing will be added to the push object. I'll need to check more closely what the different behaviours are, and which of the git behaviours makes more sense here.
carlosmn
force-pushed
the
cmn/remote-push
branch
5 times, most recently
from
a3678fc to
099bbcb
Compare
|
I've kept a no-op as valid (that is, not passing any refspecs and a remote not having any configured), since there's already a test that wants the I'm not sure whether return an error in that case would make more sense or not, tbh. |
|
@carlosmn Returning an error would be fine - same for |
|
See libgit2/rugged@d32647a63b1b^...9a5f1c9 for the rugged bindings for this. Allowed me to delete quite a bit of the old |
|
/cc @jamill should we shamelessly |
Sounds goo - I would be for pushing more logic down into lg2. As we move forward, it seems the direction is to put more of these convenience functions into lg2? For instance, would a convenience function for Pull be another candidate to put into lg2 at some point as well? |
|
Are the new callbacks also called if you use directly the low-level push APIs? |
|
New callbacks? Do you mean the copy that's in the |
|
I mean that with the current I noticed in this PR that you are adding 3 new callbacks: |
Because they're only relevant for fetching. We only receive data and messages from the server when we fetch.
Those are the copies. The last one is simply what you set for There is nothing new here, it's just wrapping the existing code inside a single function to have an equivalent api to fetch. |
But when I do |
|
None of that information comes from the server. The client is the one packing and transferring, which is why the push object has these two callbacks to give you this information. |
|
Ah sorry I missed there was a separate set of callbacks on the push object! |
This function, similar in style to git_remote_fetch(), performs all the steps required for a push, with a similar interface. The remote callbacks struct has learnt about the push callbacks, letting us set the callbacks a single time instead of setting some in the remote and some in the push operation.
We have the step-by-step method in the initialization function as we want to remove references based on the list of references which are already there, and we can use the convenience function for testing the main push.
If the user does not pass any refspecs to push, try to use those configured via the configuration or via add_push().
carlosmn
force-pushed
the
cmn/remote-push
branch
from
099bbcb to
64e3e6d
Compare
|
@jamill for things were you want to do the same simple thing 90% of the time, a lot of logic can go into libgit2. Pull is something I'm still not eager to get into the library since it's a very overloaded concept. Maybe something with a better name that won't make people think it's the |
Provide a convenience function `git_remote_push()`
6 participants
This function takes all the steps necessary to perform a push and does them for you. This is also an opportunity to provide a single callback structure instead of them being spread out between the structure and two different functions on the push object.
AFAICT basically all the bindings already provide this function themselves, so let's have an implementation in libgit2 directly.