Summary

Port Perforce backend to use run_process() instead of execute()

Review Request #14484 — Created June 27, 2025 and updated July 9, 2025, 3:47 p.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This change updates the Perforce client to use run_process() instead
of execute(). In order to simplify the implementation here, I've added
new input_string and timeout parameters to run_process and
friends, which allows passing in data to send to the command's standard
input stream.

Testing Done

Ran unit tests.

Commits

Summary	ID
Port Perforce backend to use run_process() instead of execute() This change updates the Perforce client to use `run_process()` instead of `execute()`. In order to simplify the implementation here, I've added new `input_string` and `timeout` parameters to `run_process` and friends, which allows passing in data to send to the command's standard input stream. Testing Done: Ran unit tests.	70fb055a6ae05c010b337fa44b5b0857be168aa8

Issues

Description	From	Last Updated
This can just be True now.	chipx86	June 28, 2025, 9:54 a.m.
No need for this else.	chipx86	June 28, 2025, 9:54 a.m.
Probably should be Sequence[Mapping[str, Any]].	chipx86	June 28, 2025, 9:55 a.m.
Same here.	chipx86	June 28, 2025, 10:01 a.m.
Mapping here. And similar for other functions below.	chipx86	June 28, 2025, 10:01 a.m.
This is missing a return type.	chipx86	June 28, 2025, 10:01 a.m.
This is missing a :py:class: (and then only single backticks).	chipx86	June 28, 2025, 10:01 a.m.
:py:class:	chipx86	June 28, 2025, 10:02 a.m.
Ordering here is wrong.	chipx86	June 28, 2025, 10:02 a.m.
The other arguments are (mostly) alphabetically-sorted. Can you update this to sort? That'll just keep this organized as run_process() expands.	chipx86	June 28, 2025, 10:02 a.m.
We should probably update this to use Mapping.	chipx86	June 28, 2025, 10:10 a.m.
Why this? Can we document it here and in the docstring? I feel like maybe the timeout needs to be …	chipx86	June 28, 2025, 10:10 a.m.
And timeout	maubin	July 7, 2025, 3:11 p.m.

flake8 passed.

JSHint passed.

rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This can just be True now.
```
rbtools/clients/perforce.py (Diff revision 1)
The issue has been dropped. Show all issues
```
No need for this else.
```
1. david June 28, 2025, 10:26 a.m.
  I feel like it's more clear here with it.
rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Probably should be Sequence[Mapping[str, Any]].
```
rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```
rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Mapping here. And similar for other functions below.
```
rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This is missing a return type.
```
rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This is missing a :py:class: (and then only single backticks).
```
rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
:py:class:
```
rbtools/clients/tests/test_p4.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Ordering here is wrong.
```

rbtools/utils/process.py (Diff revision 1)

The issue has been resolved. Show all issues

The other arguments are (mostly) alphabetically-sorted. Can you update this to sort? That'll just keep this organized as run_process() expands.

rbtools/utils/process.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should probably update this to use Mapping.
```

rbtools/utils/process.py (Diff revision 1)

The issue has been resolved. Show all issues

Why this? Can we document it here and in the docstring? I feel like maybe the timeout needs to be flexible too.

Description:

		This change updates the Perforce client to use `run_process()` instead
		of `execute()`. In order to simplify the implementation here, I've added
~		a new `input_string` parameter to `run_process` and friends, which
~		allows passing in data to send to the command's standard input stream.
	~	new `input_string` and `timeout` parameters to `run_process` and
	~	friends, which allows passing in data to send to the command's standard
	+	input stream.

Commits:

	Summary	ID
	Port Perforce backend to use run_process() instead of execute() This change updates the Perforce client to use `run_process()` instead of `execute()`. In order to simplify the implementation here, I've added a new `input_string` parameter to `run_process` and friends, which allows passing in data to send to the command's standard input stream. Testing Done: Ran unit tests.	055362e7605e83460438ccd8c71c5e0198033f72
	Port Perforce backend to use run_process() instead of execute() This change updates the Perforce client to use `run_process()` instead of `execute()`. In order to simplify the implementation here, I've added new `input_string` and `timeout` parameters to `run_process` and friends, which allows passing in data to send to the command's standard input stream. Testing Done: Ran unit tests.	20ec5f0d70be244bce4714e524491b5c8c0ed40f

Diff:

Revision 2 (+444 -216)

Show changes

	rbtools/clients/perforce.py
	rbtools/clients/tests/test_p4.py
	rbtools/utils/process.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

rbtools/utils/process.py (Diff revision 2)
The issue has been resolved. Show all issues
```
And timeout
```

Commits:

	Summary	ID
	Port Perforce backend to use run_process() instead of execute() This change updates the Perforce client to use `run_process()` instead of `execute()`. In order to simplify the implementation here, I've added new `input_string` and `timeout` parameters to `run_process` and friends, which allows passing in data to send to the command's standard input stream. Testing Done: Ran unit tests.	20ec5f0d70be244bce4714e524491b5c8c0ed40f
	Port Perforce backend to use run_process() instead of execute() This change updates the Perforce client to use `run_process()` instead of `execute()`. In order to simplify the implementation here, I've added new `input_string` and `timeout` parameters to `run_process` and friends, which allows passing in data to send to the command's standard input stream. Testing Done: Ran unit tests.	70fb055a6ae05c010b337fa44b5b0857be168aa8

Diff:

Revision 3 (+444 -216)

Show changes

	rbtools/clients/perforce.py
	rbtools/clients/tests/test_p4.py
	rbtools/utils/process.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```