search

libgit2 / pygit2

Python bindings for libgit2
https://www.pygit2.org/
Other
1.59k stars 383 forks source link

Add support for PackBuilder methods #1048

Closed jeremywestwood closed 3 years ago

jeremywestwood commented 3 years ago

A (very much WIP) attempt to add support for the Packbuilder methods from Libgit2 to create .pack and .idx files for a repository.

I just wanted to get initial input on whether the approach taken here is correct, or if I should be structuring it a different way?

I based the overall strategy on what was done for Libgit2Sharp: https://github.com/libgit2/libgit2sharp/pull/1183

TODOs:

Once I've got some feedback and done the TODOs then I'll write up a proper pull request.

jdavid commented 3 years ago

I don't think you need the owned parameter. Also I think you could do this only with cffi, and avoid the C code in src/, it should be much simpler; you can look at pygit2/index.py for an example of a feature (Index) done entirely in cffi. Thanks

jdavid commented 3 years ago

Looks good. Just don't see _from_c used anywhere, maybe you don't need it. I'll do a more detailed review when the tests will be available.

jeremywestwood commented 3 years ago

Thanks for the comments. I've updated based on them and added the tests.

Please let me know any other feedback.

EDIT: I see some of my tests are not working on windows, am investigating UPDATE: Should be resolved now

jdavid commented 3 years ago

Merged, thanks!