-
Notifications
You must be signed in to change notification settings - Fork 2k
Add Dulwich to Appendinx B: embedding #999
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
Conversation
Would you mind waiting until #998 merges and rebasing this? I'd like to review it on its own, and it's going to have a small merge conflict. |
Absolutely. Thank you for a kind guidance. |
339b917
to
8cf5826
Compare
Signed-off-by: Alexander Bezzubov <bzz@apache.org>
8cf5826
to
93302a1
Compare
@ben small doc on pure-python library was rebased, updated and is ready for another pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of tiny niggles.
@@ -0,0 +1,44 @@ | |||
=== Dulwich | |||
|
|||
(((Dulwich)))((("Python"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Does this index entry require the quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not know, to be hones, I just copied it from libgit2 example.
Digging deeper, #471 seems to be a reason for those quotes.
Will be happy to remove those if you think there might be some side-effects, but I guess same logic should be applicable for go-git as well.
Let me know if you want me to remove both in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, let's maybe clean them up in a separate PR then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 #1096 created
[source, python] | ||
----- | ||
from dulwich.repo import Repo | ||
r = Repo('.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind renaming this variable to current_repo
or something similar? And maybe c
should be called commit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it's just 1-to-1 copy from https://www.dulwich.io/docs/#getting-started
Please, let me know if you still think it should be change and I will be happy to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Should we make this example more readable, or easier to update? Let's stick with Dulwich's example then.
Adds one more way of embedding Git in your application using https://www.dulwich.io project.
Dulwich is independent Python implementation of the Git file formats and protocols.
As it touches the same files, it's based on/includes a commit from #998 and so will be rebased on top of it. Only ee2d9dd needs to be reviewed in here.