Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork938
Description
Theparent_commit parameter ofSubmodule.__init__ is documented this way:
GitPython/git/objects/submodule/base.py
Lines 135 to 136 inedb8d26
| :param parent_commit: | |
| See :meth:`set_parent_commit`. |
Submodule.__init__ binds this directly to the private_parent_commit attribute:
GitPython/git/objects/submodule/base.py
Line 147 inedb8d26
| self._parent_commit=parent_commit |
But this is at odds with the documented relationship toSubmodule.set_parent_commit. That method'scommit parameter corresponds to theparent_commit parameter ofSubmodule.__init__, in that itscommit parameter is used to identify the commit, if any, to set as_parent_commit. However,set_parent_commit performs both conversion and validation.
This is the relevant fragment ofset_parent_commit's docstring:
GitPython/git/objects/submodule/base.py
Lines 1253 to 1255 inedb8d26
| :param commit: | |
| Commit-ish reference pointing at the root_tree, or ``None`` to always point | |
| to the most recent commit. |
Whencommit isNone, it setsNone to_parent_commit. Otherwise, however,commit may not already be aCommit object, and that is okay, because a commit is looked up from it:
GitPython/git/objects/submodule/base.py
Line 1274 inedb8d26
| pcommit=self.repo.commit(commit) |
That's the conversion. Then validation is performed, with_parent_commit ending up set to the commit thatcommit identified only if there is such a suitable commit:
GitPython/git/objects/submodule/base.py
Lines 1275 to 1289 inedb8d26
| pctree=pcommit.tree | |
| ifself.k_modules_filenotinpctree: | |
| raiseValueError("Tree of commit %s did not contain the %s file"% (commit,self.k_modules_file)) | |
| # END handle exceptions | |
| prev_pc=self._parent_commit | |
| self._parent_commit=pcommit | |
| ifcheck: | |
| parser=self._config_parser(self.repo,self._parent_commit,read_only=True) | |
| ifnotparser.has_section(sm_section(self.name)): | |
| self._parent_commit=prev_pc | |
| raiseValueError("Submodule at path %r did not exist in parent commit %s"% (self.path,commit)) | |
| # END handle submodule did not exist | |
| # END handle checking mode |
The type annotations do not reveal the intent, as they are among those usingCommit_ish that need to be updated with the fix forCommit_ish, and that I am fixing up in#1859. My immediate motivation for opening this issue is that I'm having trouble figuring out how to annotate them, because due to the inconsistency between the docstring and the implementations, I don't know what is intended to be accepted.
Either the documentation should be updated, which could be part of#1859, or the code should be fixed to perform any expected validation and conversion and a test case added to check that this is working, which would be best done separately from#1859 (lest its scope expand ever further). I am not sure which. For#1859, it is likely sufficient for me to know what isintended, so full progress on this is not needed to finish#1859. It is my hope and also strong guess that this issue is not a blocker for#1859.
This should not be confused with#1866, which is about theparent_commits parameter ofIndexFile.commit rather than theparent_commit parameter ofSubmodule.__init__ (and which, unlike this, really is about annotations).