Adding inital types to remote.py by Yobmod · Pull Request #1229 · gitpython-developers/GitPython (original) (raw)

Thanks again for continuing this mammoth of a project of adding types to GitPython! It is much appreciated and I hope people will benefit from it soon.

Do you think this is ready for review? If so, I will see to go through it.

A few places the functions can take multiple Reference types including child and parent classes (e.g. Reference, TagReference, RemoteReference, SymbolicReference). In those cases i've used the base class that covers them all (e.g. SymbolicReference). I could change that to a union of all the classes if it would be better.

Thanks for asking, that won't be necessary. I think GitPython very much had an OO mindset at the time fo writing, so using base classes in general would be preferred where-ever possible.

I copied the compat.typing Literal and Final import logic to types.py, as it would be used in multiple places, not just compat.py. I didn't remove it from compat yet, in case there were bigger plans for it being a module.

The purpose of this module is to avoid duplicating compatibility logic and reduce affairs to a simple import. If that's not possible anymore or not necessary, I wouldn't use that module and it's certainly OK to remove bits and pieces from there if you see a chance. Thinking about it, I hope it's considered 'private' even though it's not marked as such, otherwise it wouldn't even be allowed to remove anything from it really :/.

Q. PushInfo.init() and FetchInfo() both have an 'old_commit' arg. Should they be the same type (bytes or string rather than a Commit object?) Theres a comment saying should be bytes, but that gives 14 mypy errors, whereas str gives no errors. A leftover from py2?

Actually I don't know. I could imagine that old_commit was 20bytes of SHA1 digest, but since this is remote code it won't ever see data in binary but rather strings parsed from the remote's output. It's probably OK to leave it as whatever causes the least trouble and add anything that's missing later as reports come in.