Update type for diff opcodes to be a NamedTuple.
Review Request #14398 — Created April 11, 2025 and updated
This change updates diff opcodes to use a NamedTuple with definitions
and documentation for each element.We had one annoying, confusing thing here, which is that the opcodes
returned from the differ had 5 elements, and the opcodes returned from
the opcode generator had 6 (including an extra metadata dict). I've
changed it so everything operates on the same definition, with the
metadata dict being optional in earlier stages of the process. This does
mean we have a compatibility break here for the Differ classes, which
now yield opcodes as a 6-tuple instead of a 5-tuple. The only place
that did this outside of the chunk generator was in the change
description diffs for text-based review request fields.In places where we process the opcodes, I've changed it to explicitly
use the names instead of unpacking in the loop handler. This means that
even where we're continuing to use variables likei1
andi2
, we have
an initial assignment of those that make it clearer what those variables
actually are.There's a lot of other cleanup that's going to happen in the diffviewer
code, but I wanted to keep this particular change completely separate.
Please don't worry about adjacent code that's unrelated to opcodes.
- Did a bunch of smoke testing of diff generation and saw that
everything still worked correctly. - Ran unit tests.
Summary | ID |
---|---|
e8f545ef529682136fa7007941302a08ec809899 |
- Commits:
-
Summary ID 7e657f53a8687f66341942fe88051eff41d85797 e8f545ef529682136fa7007941302a08ec809899 - Diff:
-
Revision 2 (+1322 -606)