Add types and assertions for parts of the Perforce diff logic.

Review Request #12618 — Created Sept. 22, 2022 and submitted

Information

RBTools
release-4.x

Reviewers

Perforce has some complex diff logic, and not all of it is instrumented
or even comprehensively documented based on actual usage.

In preparation for work on modernizing the diff building, this change
adds type annotations for all the _extract*() functions used to
calculate add/delete/edit/move information, and updates documentation
accordingly. To keep code readable, these functions now require keyword
arguments, and flags have defaults.

As part of this, mypy and pyright determined there were places where
None values could theoretically be passed in as paths or revisions in
some places. This would have led to broken diffs before, but shouldn't
really happen. We now assert these values as a precaution.

It was also found that revisions are sometimes passed as a string, and
sometimes as an integer. The functions themselves can handle either, but
we now formalize this in the type annotations. We may wish to
standardize it at a later date.

Unit tests pass.

Summary ID
Add types and assertions for parts of the Perforce diff logic.
Perforce has some complex diff logic, and not all of it is instrumented or even comprehensively documented based on actual usage. In preparation for work on modernizing the diff building, this change adds type annotations for all the `_extract*()` functions used to calculate add/delete/edit/move information, and updates documentation accordingly. To keep code readable, these functions now require keyword arguments, and flags have defaults. As part of this, mypy and pyright determined there were places where `None` values could theoretically be passed in as paths or revisions in some places. This would have led to broken diffs before, but shouldn't really happen. We now assert these values as a precaution. It was also found that revisions are sometimes passed as a string, and sometimes as an integer. The functions themselves can handle either, but we now formalize this in the type annotations. We may wish to standardize it at a later date.
fcdec8230adaa7baae620226c23c3d6f12da9752
Description From Last Updated

We could move this assert outside of the try block. Just makes it more consistent with the other places where …

maubinmaubin
maubin
  1. 
      
  2. rbtools/clients/perforce.py (Diff revision 1)
     
     

    We could move this assert outside of the try block. Just makes it more consistent with the other places where you added assertions.

  3. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.x (f869776)
Loading...