Update ParsedDiffFile instantiation to take a ParsedDiffChange.

Review Request #11745 — Created July 27, 2021 and submitted

Information

Review Board
release-4.0.x

Reviewers

Now that ParsedDiffFile expects a ParsedDiffChange argument, it's
time to update our diff parsers to provide.

The base DiffParser handles this for most of our diff parsers. We only
need to be careful to discard the parser if skipping the file. This
requires a little bit of code reorganization, but not much.

GitDiffParser also creates one, but this change is a bit easier. We
simply need to make sure we don't bail immediately after creating one,
if we don't have enough expected lines remaining in the diff file. While
here, I've taken the opportunity to clean up some comments and add
docstrings.

There is an important change in usage of ParsedDiffChange now. If a
diff parser constructs its own instances (which most won't need to do),
it can no longer just throw it away. It'll still be in the list of
files. It must now call discard() to get rid of it. This isn't an
issue in Review Board or Power Pack, and is unlikely to impact anyone in
production, given how uncommon this is.

Unit tests pass.

Summary ID
Update ParsedDiffFile instantiation to take a ParsedDiffChange.
Now that `ParsedDiffFile` expects a `ParsedDiffChange` argument, it's time to update our diff parsers to provide. The base `DiffParser` handles this for most of our diff parsers. We only need to be careful to discard the parser if skipping the file. This requires a little bit of code reorganization, but not much. `GitDiffParser` also creates one, but this change is a bit easier. We simply need to make sure we don't bail immediately after creating one, if we don't have enough expected lines remaining in the diff file. While here, I've taken the opportunity to clean up some comments and add docstrings. There is an important change in usage of `ParsedDiffChange` now. If a diff parser constructs its own instances (which most won't need to do), it can no longer just throw it away. It'll still be in the list of files. It must now call `discard()` to get rid of it. This isn't an issue in Review Board or Power Pack, and is unlikely to impact anyone in production, given how uncommon this is.
407c6a4d02b1d8904bf4e82f1bb9f4259583b9cc
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (9052dfc)
Loading...