Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

EliahKagan · 2024-01-24T04:28:01Z

This clarifies the comment that explains the significance of setting the Git.GIT_PYTHON_GIT_EXECUTABLE attribute when running Git failed but the attribute wasn't set before.

This happens in the code path in Git.refresh where has_git is False and old_git is None, provided the refresh mode doesn't call for the initial refresh to raise an exception on failure.

It was previously described in terms of a "first" and "second" import, but as discussed in #1804, the rationale and effect were not altogether clear. This uses wording along the lines of what I suggested there, adjusted a bit for context.

This clarifies the comment that explains the significance of setting the Git.GIT_PYTHON_GIT_EXECUTABLE attribute when running Git failed but the attribute wasn't set before. This happens in the code path in Git.refresh where has_git is False and old_git is None, provided the refresh mode doesn't call for the initial refresh to raise an exception on failure. It was previously described in terms of a "first" and "second" import, but as discussed in gitpython-developers#1804, the rationale and effect were not altogether clear.

Byron

Thanks a lot for not giving up on finding an explanation for the previous (and unclear) wording in order to find the right way to improve it :).

git/cmd.py

I think this refactoring slightly improves clarity. + Condense/sort/group imports (while adding contextlib import). + Minor readability/style tweaks in FetchInfo.refresh comments. + Make a comment in Git.refresh clearer, building on revisions in: gitpython-developers#1810 (comment)

I think this refactoring slightly improves readability. + Condense/sort/group imports (while adding contextlib import). + Minor style tweaks in FetchInfo.refresh comments. + Make a comment in Git.refresh clearer, building on revisions in: gitpython-developers#1810 (comment)

EliahKagan mentioned this pull request Jan 24, 2024

Explanation makes unclear reference to "first" and "second" imports #1804

Closed

EliahKagan force-pushed the refresh branch from e80fc5f to a11b89a Compare January 24, 2024 05:11

EliahKagan changed the title ~~Clarify why an initial failing git command can be set~~ Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure Jan 24, 2024

Byron approved these changes Jan 24, 2024

View reviewed changes

git/cmd.py Outdated Show resolved Hide resolved

git/cmd.py Outdated Show resolved Hide resolved

Apply suggestions from code review

82cbc6c

Byron merged commit 307f198 into gitpython-developers:main Jan 24, 2024
10 checks passed

EliahKagan deleted the refresh branch January 24, 2024 07:52

EliahKagan mentioned this pull request Jan 25, 2024

Report actual attempted Git command when Git.refresh fails #1812

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

EliahKagan commented Jan 24, 2024 •

edited

Loading

Byron left a comment

Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

Conversation

EliahKagan commented Jan 24, 2024 • edited Loading

Byron left a comment

Choose a reason for hiding this comment

EliahKagan commented Jan 24, 2024 •

edited

Loading