GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Add initial types to repo/fun.py by Yobmod · Pull Request #1192 · gitpython-developers/GitPython Conversation
|
|
Finally fixed all automated issues! All functions in base.py and fun.py now typed and passes mypy and pyright (and flake8) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this tremendous work!
Admittedly I only got to fly through about 2/3 of all changes and think there is a high likelihood of me having missed something.
I will see to finish the rest tomorrow but would hope that another maintainer could also have a look.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| GitCommandWrapperType = Git | ||
|
|
||
| def __init__(self, path=None, odbt=GitCmdObjectDB, search_parent_directories=False, expand_vars=True): | ||
| def __init__(self, path: Optional[PathLike] = None, odbt: Type[GitCmdObjectDB] = GitCmdObjectDB, |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
There is an actual base class for object databases which is more general.
There are two implementations for it.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LooseObjectDB?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think it's best to 'make up' a type and call it ObjectDB, as LooseObjectDB is too specific and kind of wrong here, too. Honestly I don't really know what I was thinking and this clearly is an example of OOP being gone wrong.
Otherwise you could dig into GitDB and see how this DB is composed of various types which provide a bundle of methods which may or may not be that useful in practice.
I do recommend to not get into types of GitDB and instead make them up (inside of GitPython) based on what's common usage in GitPython instead.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| return Head.create(self, path, commit, force, logmsg) | ||
|
|
||
| def delete_head(self, *heads, **kwargs): | ||
| def delete_head(self, *heads: HEAD, **kwargs: Any) -> None: |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I think HEAD is too specific - any symbolic reference can be created that way.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
amended in #1202
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| elif config_level == "global": | ||
| return osp.normpath(osp.expanduser("~/.gitconfig")) | ||
| elif config_level == "repository": | ||
| return osp.normpath(osp.join(self._common_dir or self.git_dir, "config")) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It's probably nicer to do it like this:
if not (self._common_dir or self.git_dir): raise … return osp.normpath(osp.join(self._common_dir or self.git_dir, "config"))
That way there is no duplicate path transformation.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
amended in #1202
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
As this PR touches a lot of files I am merging it nonetheless even without the requested changes. These can be contributed separately and in the meantime what's there should already be a considerable improvement. Thanks again, and I am looking forward to your future contributions :). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry for slow reply, I was sent to work somewhere with no internet (shocking common in the UK!) I'll make the suggested changes on next PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.