Skip to content

Clarify Git.execute and Popen arguments #1688

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.

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

Merged
merged 11 commits into from
Oct 3, 2023
Prev Previous commit
Next Next commit
Avoid having a local function seem to be a method
The kill_process local function defined in the Git.execute method
is a local function and not a method, but it was easy to misread as
a method for two reasons:

- Its docstring described it as a method.

- It was named with a leading underscore, as though it were a
  nonpublic method. But this name is a local variable, and local
  variables are always nonpublic (except when they are function
  parameters, in which case they are in a sense public). A leading
  underscore in a local variable name usually means the variable is
  unused in the function.

This fixes the docstring and drops the leading underscore from the
name. If this is ever extracted from the Git.execute method and
placed at class or module scope, then the name can be changed back.
  • Loading branch information
EliahKagan committed Oct 3, 2023
commit fc755dae6866b9c9e0aa347297b693fec2c5b415
6 changes: 3 additions & 3 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,8 +1012,8 @@ def execute(
if as_process:
return self.AutoInterrupt(proc, command)

def _kill_process(pid: int) -> None:
"""Callback method to kill a process."""
def kill_process(pid: int) -> None:
"""Callback to kill a process."""
p = Popen(
["ps", "--ppid", str(pid)],
stdout=PIPE,
Expand Down Expand Up @@ -1046,7 +1046,7 @@ def _kill_process(pid: int) -> None:

if kill_after_timeout is not None:
kill_check = threading.Event()
watchdog = threading.Timer(kill_after_timeout, _kill_process, args=(proc.pid,))
watchdog = threading.Timer(kill_after_timeout, kill_process, args=(proc.pid,))

# Wait for the process to return
status = 0
Expand Down