• 
      

    Update type for diff opcodes to be a NamedTuple.

    Review Request #14398 — Created April 11, 2025 and discarded

    Information

    Review Board
    master

    Reviewers

    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 like i1 and i2, 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
    Update type for diff opcodes to be a NamedTuple.
    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 like `i1` and `i2`, 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. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.
    e8f545ef529682136fa7007941302a08ec809899
    Description From Last Updated

    Missing documentation.

    maubinmaubin
    david
    maubin
    1. 
        
    2. reviewboard/diffviewer/processors.py (Diff revision 2)
       
       
       
       
       
       
       
      Show all issues

      Missing documentation.

    3. 
        
    david
    Review request changed
    Status:
    Discarded