Fix name collision #2060
Fix name collision #2060
Conversation
|
Thanks, this PR would be great to land I am sure! Do you have an intuition as to why CI is failing here? |
|
Yes. There was an rst syntax error in CPython's configparser module: https://github.com/python/cpython/blob/v3.8.10/Lib/configparser.py#L768: - `vars'
+ `vars`configparser.RawConfigParser is the baseclass of This only occurs in the pipelines for 3.8 and 3.9, so I guess it was fixed later but the fix was never backported. I'm not sure what's the best way to handle this. I can think of multiple options, but I dislike all of them:
|
|
Here is how this would render without It is not beautiful, but people will understand it. If I understand correctly, this project is mostly in maintenance-only mode, so the risk of such issues being newly introduced to GitPython itself is fairly low. Maybe that's the best of all the bad options? |
|
Sorry for causing this mess, I didn't expect any negative side effects from the bugfix. If you want to close this PR, just go ahead, I don't have strong feeling about this and just wanted to contribute a quick win. If you want me to investigate further in a specific direction, please let me know and I'll see what is feasible for me. Thanks for this library, btw! :) |
|
I see - the fix now causes the configuration to be pulled in correctly, and that results in a conflict in the way a docstring is written in the library or code we pull in. The issue itself, now that I understand it, is also amazing as it's one of these inconsistencies that shouldn't even be possible. How can a variable in a for-loop survive the for-loop, and permanently overwrite a variable in the outer scope? Let's go with the fix, and let only errors be errors during documentation generation. Just one more request: could you add to the body of the most recent commit why the change was made? Just copy-paste a link to the discussion here, or better yet, copy-paste the respective comment text. Thanks again. Here is a copilot suggestion, but it might lack detail. |
cb77830 to
5e7d005
Compare
Workaround for python/cpython#100520 (rst syntax error in configparser docstrings), which was fixed in CPython 3.10+. Docutils raises warnings about the invalid docstrings, and `-W` instructs sphinx to treat this as errors. We can't control or silence these warnings, so we accept them and don't treat them as errors. See the discussion in gitpython-developers#2060 for details.
Workaround for python/cpython#100520 (rst syntax error in configparser docstrings), which was fixed in CPython 3.10+. Docutils raises warnings about the invalid docstrings, and `-W` instructs sphinx to treat this as errors. We can't control or silence these warnings, so we accept them and don't treat them as errors. See the discussion in gitpython-developers#2060 for details.
5e7d005 to
80fd2c1
Compare
|
Should be good now, please let me know if there is more to do. |
|
Thank you, I think the PR is ready to go now! |
The bug
In
git.config.MetaParserBuilder.__new__:Downstream effects
git.config.GitConfigParser.__name__is"write", not"GitConfigParser"git.config.GitConfigParseras an alias of itself:Fixes #2023