Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move remote callbacks to an interface/object #568

Merged
merged 4 commits into from
Sep 27, 2015

Conversation

carlosmn
Copy link
Member

Instead of setting the methods/fields in the remote itself, which is a vestige of the older method which libgit2 used, have the user pass in an object with their callbacks, which represents what's going on underneath much more accurately, as well as letting us re-use the callbacks implementation across Remote and clone_repsitory().

The docs aren't updated yet, so it's still WIP, but this is what I'd like us to move to.

@jdavid
Copy link
Member

jdavid commented Sep 26, 2015

👍

We could support another, less verbose, way to write. Now:

class MyCallbacks(pygit2.RemoteCallbacks):
    def __init__(self):
        self.credentials = pygit2.UserPass("libgit2", "libgit2")

callbacks = MyCallbacks()

Or less verbose:

callbacks = pygit2.RemoteCallbacks(
    credentials=pygit2.UserPass("libgit2", "libgit2"),
)

@carlosmn
Copy link
Member Author

That's a good idea. The credentials or certificate callbacks are some which you sometimes must provide in order to complete the operation, so it makes sense to make them easier to provide in-line.

@carlosmn carlosmn changed the title [WIP] Move remote callbacks to an interface/object Move remote callbacks to an interface/object Sep 26, 2015
@carlosmn carlosmn changed the title Move remote callbacks to an interface/object [WIP] Move remote callbacks to an interface/object Sep 26, 2015
This represents what's going on much better than the remnants from the
older methods. What we do is pass a list of callbacks to libgit2 for it
to call, and they are valid for a single operation, not for the remote
itself.

This should also make it easier to re-use callbacks which have already
been set up.
This lets use the same callbacks for fetch and clone; it also fills in
the callbacks which the clone function did not support.
This allows for a less verbose way of setting one-liners as these
callbacks.
This was implemented for clone, but not for fetch or push, so it was
deleted during the conversion, which shows why we need to unify these
callback structures.
@carlosmn carlosmn changed the title [WIP] Move remote callbacks to an interface/object Move remote callbacks to an interface/object Sep 27, 2015
@carlosmn
Copy link
Member Author

This should now give us parity in both. Through the unification we've gained the progress callbacks on clone and the certificate check on fetch and push, instead of having to implement each of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants