This change has been marked as completed.

Describe the completed change (optional):

Pushed to master (72e5de9)

Summary

Wrap git invocations in `with`.

Review Request #14400 — Created April 14, 2025 and submitted 1 month, 2 weeks ago

Information

Owner

david*

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description*

Running with the latest Python, I was getting warnings about leaked file
descriptors, which were coming from git invocations inside the
GitClient, as it created BufferedReader instances for stdout/stderr.
This change wraps those subprocess invocations in with blocks, causing
things to get cleaned up appropriately.

Testing Done

Loaded some diffs and no longer saw warnings about leaked resources.

Commits

Summary	ID
Wrap git invocations in `with`. Running with the latest Python, I was getting warnings about leaked file descriptors, which were coming from `git` invocations inside the `GitClient`, as it created `BufferedReader` instances for stdout/stderr. This change wraps those subprocess invocations in `with` blocks, causing things to get cleaned up appropriately. Testing Done: Loaded some diffs and no longer saw warnings about leaked resources.	7351b24206b3a0d608bda2a4abef740730be3a2c

Summary

Wrap git invocations in `with`.

Running with the latest Python, I was getting warnings about leaked file descriptors, which were coming from `git` invocations inside the `GitClient`, as it created `BufferedReader` instances for stdout/stderr. This change wraps those subprocess invocations in `with` blocks, causing things to get cleaned up appropriately. Testing Done: Loaded some diffs and no longer saw warnings about leaked resources.

7351b24206b3a0d608bda2a4abef740730be3a2c

Issues

Description	From	Last Updated
Can you change '--git-dir=%s' to an f-string?	maubin	2 months ago
There are no open issues

flake8 passed.

JSHint passed.

reviewboard/scmtools/git.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you change '--git-dir=%s' to an f-string?
```

Commits:

	Summary	ID
	Wrap git invocations in `with`. Running with the latest Python, I was getting warnings about leaked file descriptors, which were coming from `git` invocations inside the `GitClient`, as it created `BufferedReader` instances for stdout/stderr. This change wraps those subprocess invocations in `with` blocks, causing things to get cleaned up appropriately. Testing Done: Loaded some diffs and no longer saw warnings about leaked resources.	689b186907c59be8661fd538f166b17a50826553
	Wrap git invocations in `with`. Running with the latest Python, I was getting warnings about leaked file descriptors, which were coming from `git` invocations inside the `GitClient`, as it created `BufferedReader` instances for stdout/stderr. This change wraps those subprocess invocations in `with` blocks, causing things to get cleaned up appropriately. Testing Done: Loaded some diffs and no longer saw warnings about leaked resources.	7351b24206b3a0d608bda2a4abef740730be3a2c

Diff:

Revision 2 (+28 -22)

Show changes

reviewboard/scmtools/git.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (72e5de9)

reviewboard/scmtools/git.py (Diff revision 1)