René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Submodule.__init__ `parent_commit` conversion/validation is implied but not done","articleBody":"The `parent_commit` parameter of `Submodule.__init__` is documented this way:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/edb8d26e20d16f72240f7040c353d01a3a1a9740/git/objects/submodule/base.py#L135-L136\r\n\r\n`Submodule.__init__` binds this directly to the private `_parent_commit` attribute:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/edb8d26e20d16f72240f7040c353d01a3a1a9740/git/objects/submodule/base.py#L147\r\n\r\nBut this is at odds with the documented relationship to `Submodule.set_parent_commit`. That method's `commit` parameter corresponds to the `parent_commit` parameter of `Submodule.__init__`, in that its `commit` parameter is used to identify the commit, if any, to set as `_parent_commit`. However, `set_parent_commit` performs both conversion and validation.\r\n\r\nThis is the relevant fragment of `set_parent_commit`'s docstring:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/edb8d26e20d16f72240f7040c353d01a3a1a9740/git/objects/submodule/base.py#L1253-L1255\r\n\r\nWhen `commit` is `None`, it sets `None` to `_parent_commit`. Otherwise, however, `commit` may not already be a `Commit` object, and that is okay, because a commit is looked up from it:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/edb8d26e20d16f72240f7040c353d01a3a1a9740/git/objects/submodule/base.py#L1274\r\n\r\nThat's the conversion. Then validation is performed, with `_parent_commit` ending up set to the commit that `commit` identified only if there is such a suitable commit:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/edb8d26e20d16f72240f7040c353d01a3a1a9740/git/objects/submodule/base.py#L1275-L1289\r\n\r\nThe type annotations do not reveal the intent, as they are among those using `Commit_ish` that need to be updated with the fix for `Commit_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.\r\n\r\nEither 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 is *intended*, 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.\r\n\r\nThis should not be confused with #1866, which is about the `parent_commits` parameter of `IndexFile.commit` rather than the `parent_commit` parameter of `Submodule.__init__` (and which, unlike this, really is about annotations).","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2024-03-12T20:22:33.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":8},"url":"https://github.com/1869/GitPython/issues/1869"}