Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure #1810

merged 2 commits into from
Jan 24, 2024

Conversation

EliahKagan
Copy link
Contributor

@EliahKagan EliahKagan commented Jan 24, 2024 •
edited
Loading

Fixes #1804

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.

@EliahKagan EliahKagan mentioned this pull request Jan 24, 2024
Closed
@EliahKagan
Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure
a11b89a 
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.
@EliahKagan 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
Byron approved these changes Jan 24, 2024
View reviewed changes
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Outdated Show resolved Hide resolved
git/cmd.py Outdated Show resolved Hide resolved
@Byron Byron merged commit 307f198 into gitpython-developers:main Jan 24, 2024
10 checks passed
@EliahKagan EliahKagan deleted the refresh branch January 24, 2024 07:52
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request Jan 25, 2024
@EliahKagan
Condense FetchInfo.refresh using contextlib.suppress
3c406ae 
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)
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request Jan 25, 2024
@EliahKagan
Condense FetchInfo.refresh using contextlib.suppress
e8a2ac0 
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 added a commit to EliahKagan/GitPython that referenced this pull request Jan 25, 2024
@EliahKagan
Condense FetchInfo.refresh using contextlib.suppress
3bb63f3 
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 EliahKagan mentioned this pull request Jan 25, 2024
Merged
lettuce-bot bot added a commit to lettuce-financial/github-bot-signed-commit that referenced this pull request Feb 15, 2024
@lettuce-bot
Update dependency GitPython to v3.1.42 (#54)
02dd6cd 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [GitPython](https://github.com/gitpython-developers/GitPython) |
`==3.1.41` -> `==3.1.42` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.41/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.41/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>gitpython-developers/GitPython (GitPython)</summary>

###
[`v3.1.42`](https://github.com/gitpython-developers/GitPython/releases/tag/3.1.42)

[Compare
Source](https://github.com/gitpython-developers/GitPython/compare/3.1.41...3.1.42)

#### What's Changed

- Fix release link in changelog by
[@&#8203;PeterJCLaw](https://github.com/PeterJCLaw) in
[gitpython-developers/GitPython#1795
- Remove test dependency on sumtypes library by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1798
- Pin Sphinx plugins to compatible versions by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1803
- fix: treeNotSorted issue by
[@&#8203;et-repositories](https://github.com/et-repositories) in
[gitpython-developers/GitPython#1799
- Remove git.util.NullHandler by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1807
- Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1810
- Report actual attempted Git command when Git.refresh fails by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1812
- Don't suppress messages when logging is not configured by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1813
- Pin Python 3.9.16 on Cygwin CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1814
- Have initial refresh use a logger to warn by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1815
- Omit warning prefix in "Bad git executable" message by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1816
- Test with M1 macOS CI runner by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1817
- Bump pre-commit/action from 3.0.0 to 3.0.1 by
[@&#8203;dependabot](https://github.com/dependabot) in
[gitpython-developers/GitPython#1818
- Bump Vampire/setup-wsl from 2.0.2 to 3.0.0 by
[@&#8203;dependabot](https://github.com/dependabot) in
[gitpython-developers/GitPython#1819
- Remove deprecated section in README.md by
[@&#8203;marcm-ml](https://github.com/marcm-ml) in
[gitpython-developers/GitPython#1823
- Keep temp files out of project dir and improve cleanup by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1825

#### New Contributors

- [@&#8203;PeterJCLaw](https://github.com/PeterJCLaw) made their first
contribution in
[gitpython-developers/GitPython#1795
- [@&#8203;et-repositories](https://github.com/et-repositories) made
their first contribution in
[gitpython-developers/GitPython#1799
- [@&#8203;marcm-ml](https://github.com/marcm-ml) made their first
contribution in
[gitpython-developers/GitPython#1823

**Full Changelog**:
gitpython-developers/GitPython@3.1.41...3.1.42

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/lettuce-financial/github-bot-signed-commit).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
renovate bot added a commit to allenporter/flux-local that referenced this pull request Feb 16, 2024
@renovate
chore(deps): update dependency gitpython to v3.1.42 (#550)
f061cde 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [GitPython](https://github.com/gitpython-developers/GitPython) |
`==3.1.41` -> `==3.1.42` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.41/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.41/3.1.42?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>gitpython-developers/GitPython (GitPython)</summary>

###
[`v3.1.42`](https://github.com/gitpython-developers/GitPython/releases/tag/3.1.42)

[Compare
Source](https://github.com/gitpython-developers/GitPython/compare/3.1.41...3.1.42)

#### What's Changed

- Fix release link in changelog by
[@&#8203;PeterJCLaw](https://github.com/PeterJCLaw) in
[gitpython-developers/GitPython#1795
- Remove test dependency on sumtypes library by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1798
- Pin Sphinx plugins to compatible versions by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1803
- fix: treeNotSorted issue by
[@&#8203;et-repositories](https://github.com/et-repositories) in
[gitpython-developers/GitPython#1799
- Remove git.util.NullHandler by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1807
- Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1810
- Report actual attempted Git command when Git.refresh fails by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1812
- Don't suppress messages when logging is not configured by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1813
- Pin Python 3.9.16 on Cygwin CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1814
- Have initial refresh use a logger to warn by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1815
- Omit warning prefix in "Bad git executable" message by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1816
- Test with M1 macOS CI runner by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1817
- Bump pre-commit/action from 3.0.0 to 3.0.1 by
[@&#8203;dependabot](https://github.com/dependabot) in
[gitpython-developers/GitPython#1818
- Bump Vampire/setup-wsl from 2.0.2 to 3.0.0 by
[@&#8203;dependabot](https://github.com/dependabot) in
[gitpython-developers/GitPython#1819
- Remove deprecated section in README.md by
[@&#8203;marcm-ml](https://github.com/marcm-ml) in
[gitpython-developers/GitPython#1823
- Keep temp files out of project dir and improve cleanup by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[gitpython-developers/GitPython#1825

#### New Contributors

- [@&#8203;PeterJCLaw](https://github.com/PeterJCLaw) made their first
contribution in
[gitpython-developers/GitPython#1795
- [@&#8203;et-repositories](https://github.com/et-repositories) made
their first contribution in
[gitpython-developers/GitPython#1799
- [@&#8203;marcm-ml](https://github.com/marcm-ml) made their first
contribution in
[gitpython-developers/GitPython#1823

**Full Changelog**:
gitpython-developers/GitPython@3.1.41...3.1.42

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Byron Byron Byron approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Explanation makes unclear reference to "first" and "second" imports
2 participants
@EliahKagan @Byron