René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"RemoteProgress._parse_progress_line may merit a broader return type","articleBody":"`RemoteProgress._parse_progress_line` is conceptually a \"protected\" method: code outside GitPython may subclass `RemoteProgress` and override it with another implementation (which may also reasonably call the base class implementation).\r\n\r\nThe method is [conceptually void](https://github.com/gitpython-developers/GitPython/pull/1755) as implemented in the base class, always implicitly returning `None`, and it is annotated accordingly:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/afa575454f85b34800460881cc9c3cd7fe78c8e3/git/util.py#L582\r\n\r\nHowever, while it would not ordinarily be useful to do so, it is--or *has* been--also intended that it be overridable to return a non-`None` value. This is mostly for compatibility with existing code that may have done so in the past, but it may be that reasonable cases for doing so in new code can still arise. This is reflected in code of GitPython:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/afa575454f85b34800460881cc9c3cd7fe78c8e3/git/util.py#L685-L697\r\n\r\nAnd in the subclass of `RemoteProgress` introduced in the test suite, which overrides the method:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/afa575454f85b34800460881cc9c3cd7fe78c8e3/test/test_remote.py#L54-L59\r\n\r\nBut any override that returns a non-`None` value is currently inconsistent with the return type annotation on the base class, which is written as `-\u003e None`. For Liskov substitutability, an overridden method's return type must be the same or a subclass of the base method's return type, and if a subclass has type annotations, and the return type is specified as a non-`Any` type, type checkers such as `mypy` and `pyright` will report a non-`None` return type as a violation.\r\n\r\nIf possible, the return type in `RemoteProgress` should be broadened, but in a way that does not unduly encourage subclasses to return non-`None`. **See comments on #1788, starting at https://github.com/gitpython-developers/GitPython/pull/1788#issuecomment-1869653326, for a discussion and analysis of this.** This issue, and the above recommendation, is based on the findings and discussion there.","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2024-02-20T23:39:26.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/1834/GitPython/issues/1834"}