Models

There are two code review models: pre-commit and post-commit. Which model you use for code reviews with P4 Code Review is up to you.

Pre-commit model

The pre-commit code review model allows developers to review code before it is committed. This allows reviewers to look at code early and identify potential issues before they reach production environments.

P4 Code Review uses the shelving feature in P4 Server for a pre-commit code reviews. For more information on shelving, see Shelve Changelists in P4 CLI Documentation.

Use P4 Code Review to enforce code review procedures and ensure that code is approved before it is committed.

Post-commit model

In the post-commit code review model, code is committed to the P4 Server before a code review is initiated. This approach does not involve shelving and avoids blocking contributors, as developers are not required to wait for an approved review before committing their changes.

However, this model can make it more difficult to catch and correct errors, especially in environments with continuous integration (CI), since issues may not be identified until after code has been merged. As a result, there is a greater risk that new work may be built upon faulty or unreviewed code, which can complicate debugging and reduce overall code quality.

Internal representation

P4 Code Review-managed changelists

A code review consists of one or more shelved changelists that P4 Code Review manages. A shelved changelist is a pending changelist that has a snapshot of its files on a shelf associated with the changelist.

When a review is started, P4 Code Review creates a new changelist that becomes the review changelist. What happens afterwards varies:

• If the review contains uncommitted work (the pre-commit model), P4 Code Review copies the shelved files from the user's changelist that initiated the review into the review's changelist.
• Any time that a user's changelist associated with the review has its shelved files updated, P4 Code Review copies the shelved files into its review changelist and creates an archive changelist. An archive changelist is no different from any other pending changelist with shelved files, but it allows P4 Code Review to provide versioning and diffs within a review.
• If the head version of a review is committed (the post-commit model), the review's changelist is emptied of files.

The review's changelist is never actually committed; this allows the review to be opened later with additional shelved changes.

$ p4 changelists -u swarm
Change 1212285 on 2015/07/31 by swarm@swarm-96017af4-5615-9819-7af1-6fc1fa537214 *pending* 'Add requirements and instructions'
Change 1212284 on 2015/07/31 by swarm@swarm-96017af4-5615-9819-7af1-6fc1fa537214 *pending* 'Add requirements and instructions'
...

P4 Code Review clients

Whenever P4 Code Review creates a changelist for a review, it uses a client workspace (or clients) associated with the configured P4 Server userid that has admin privileges. Whenever a user commits a change via P4 Code Review's user interface, P4 Code Review uses a client associated with that user.

These clients are named swarm-{uuid}, for example swarm-5ad4a9c0-06e7-20eb-897f-cbd4cc934295. To learn more about clients, see P4 Server as a version control implementation in the P4 Overview Guide.

The client that P4 Code Review creates and uses live in the SWARM_ROOT/data/clients folder.

Inside the clients folder, P4 Code Review maintains a user-specific folder that contains any client folders that may be required. Each user-specific folder is named by converting their P4 Server userid into hexadecimal to avoid any characters that would be problematic in the filesystem, such as slashes, accents, UTF-8 characters, etc. For example, the folder for the user steve.russell would be named 73746576652e72757373656c6c.

Within the user-specific folder are the folders that become the root of each client. Each of these folders is named with a globally-unique identifier (GUID) prefixed with swarm-, for example swarm-438d482b-f107-9a35-c06c-86ac68136b00. Accompanying each folder is a lock file with the same name plus the .lock extension. Finally, the user-specific clients folder contains a management lock file called manage.lock.

Here is an example of the folder structure:

SWARM_ROOT/
	data/
		clients/
		  73746576652e72757373656c6c/
			manage.lock
			swarm-438d482b-f107-9a35-c06c-86ac68136b00/
			swarm-438d482b-f107-9a35-c06c-86ac68136b00.lock
			swarm-8388362a-233d-0cb9-3e90-895eaaa99f6c/
			swarm-8388362a-233d-0cb9-3e90-895eaaa99f6c.lock
		  7061756c612e626f796c65/
			manage.lock
			swarm-da7de4b4-0ecb-12c8-1b35-f3e32bb18033/
			swarm-da7de4b4-0ecb-12c8-1b35-f3e32bb18033.lock

Here are the steps P4 Code Review takes when it needs to use a client:

1. Convert the current connection's userid to hexadecimal.
2. Check to see whether a user-specific folder exists within SWARM_ROOT/data/clients; if not, create the folder.

3. Within the user-specific folder, loop over any existing client folders and attempt to lock each in turn:

   If a lock is acquired skip to the next step. Otherwise, perform the following procedure.

   Create client procedure:

   1. Check if the max number of clients for the current user has been reached:

      • If so, wait a short amount of time (50 milliseconds), and start step 3 again.
      • If not, proceed to the next step.
   2. Take control of a lock on manage.lock.

   3. Check if the max number of clients for the current user has been reached:

      • If so, release the manage.lock, wait a short amount of time (50 milliseconds), and start step 3 again.
      • If not, proceed to the next step.
   4. Create a new client folder using a GUID-based filename, and take a lock on the folder.
   5. Release the manage.lock lock.
4. Perform the necessary file operations using the locked client folder.

5. Revert the file content within the client folder to avoid having constantly growing disk space use.

   There may occasionally be stray files left; P4 Code Review is not aggressive about cleaning up.

6. P4 Code Review releases the lock on the client folder.

Most users should only require 1-2 clients, and those are only required if they commit from P4 Code Review. The admin user that P4 Code Review is configured to use should only use one client per configured worker.

By default, the number of clients that could be active at any given instant is two times the number of configured workers. Since the default worker count is three, P4 Code Review would use at most six clients simultaneously.

If the client limit is reached, further file processing is blocked until a client becomes available. Potentially, this means that users could encounter timeouts. Configuring P4 Code Review to use more workers could solve that issue.

Removal considerations when deleting P4 Code Review clients

There are a few considerations that should be assessed prior to removal:

• Ideally, you should stop the web server (taking P4 Code Review out of service) before removing a client from the P4 Code Review server; this eliminates the risk of removing a client that is in use.

  If you do not stop the web server first, P4 Code Review may encounter an error during a commit.

• Removal of a client folder does not remove the client spec from the P4 Server. Unless the client spec is removed, that client effectively becomes orphaned. Orphaned clients are, of themselves, not a big concern as the storage and performance impact is negligible.

• Removal of a client’s corresponding spec in the P4 Server can be done. However, you should never remove a client spec that has associated shelved files.

  Usually, the only client specs that should have associated shelved files belong to the admin account that P4 Code Review is configured to use. All other clients that may exist for other users are primarily used for committing changes, and so should not have shelved files associated.

• Anything owned by the P4 Code Review client will become inaccessible to users if the client is deleted. This includes reviews and changelists.