Summary:

Clean up Perforce change description rewriting and fix newline errors.

Review Request #13023 — Created May 8, 2023 and submitted May 15, 2023, 2:50 p.m.

Information
Owner:	chipx86
Repository:	RBTools
Branch:	release-4.x
Bugs:
Depends On:
Reviewers
Groups:	rbtools
People:

Description

We've had code in the tree for a long while to stamp Perforce changes
with a review request URL, but it had a flaw. It left out a newline,
causing the first line to read:

Description:   Here is the first line.
        And here is the second line.

This was due to a missing newline after adding the "Description:" field.

This change fixes this up, reworks the logic and comments to be a bit
more clear about the states being kept and the newlines added, and adds
unit tests.

It also fixes up the amending portion, which didn't include a changenum
in the p4 change command (which meant it couldn't amend anything but
default).

A timeout is now set for the p4 change command, in case anything fails
during a hook script. Errors are outputted if anything goes wrong.

Testing Done

Unit tests passed.

Tested manually with updates to the trigger script.

Commits

Summary
	Clean up Perforce change description rewriting and fix newline errors.

Screenshots

Issues

Description	From	Last Updated
undefined name 'changenum' Column: 12 Error code: F821	reviewbot	May 8, 2023, 12:35 p.m.
undefined name 'changenum' Column: 25 Error code: F821	reviewbot	May 8, 2023, 12:35 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/clients/perforce.py (Diff revision 1)

Show all issues

undefined name 'changenum'

Column: 12
Error code: F821

rbtools/clients/perforce.py (Diff revision 1)

Show all issues

undefined name 'changenum'

Column: 25
Error code: F821

Change Summary:


Added a couple missing changes to the commit for the renamed call and a function definition update.
Fleshed out the description to cover the related changes.

Description:

		We've had code in the tree for a long while to stamp Perforce changes
		with a review request URL, but it had a flaw. It left out a newline,
		causing the first line to read:



		`Description: Here is the first line.`
		And here is the second line.




		This was due to a missing newline after adding the "Description:" field.

		This change fixes this up, reworks the logic and comments to be a bit
		more clear about the states being kept and the newlines added, and adds
		unit tests.
	+
	+	It also fixes up the amending portion, which didn't include a changenum
	+	in the `p4 change` command (which meant it couldn't amend anything but
	+	`default`).
	+
	+	A timeout is now set for the `p4 change` command, in case anything fails
	+	during a hook script. Errors are outputted if anything goes wrong.

Commits:

	Summary
-		Clean up Perforce change description rewriting and fix newline errors.
+		Clean up Perforce change description rewriting and fix newline errors.

Diff:

Revision 2 (+492 -80)

Show changes

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

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-4.x (f6e1d5a)