This change has been marked as completed.

Describe the completed change (optional):

Pushed to master (df24e42)

Summary

Add typing for diff opcodes.

Review Request #14418 — Created May 2, 2025 and submitted 1 week, 6 days ago

Information

Owner

david*

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description*

This change updates diff opcodes to use explicitly typed tuples. The
opcodes created by the Differ implementation have a tag and ranges,
and then the opcode generator adds an additional metadata field. In the
interest of avoiding any changes, we therefore have separate definitions
for both steps.

Testing Done


Did a bunch of smoke testing of diff generation and saw that

  everything still worked correctly.
Ran unit tests.

Commits

Summary	ID
Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f8ad232f1ebd19142135457de29d0b3156da9519

Summary

Add typing for diff opcodes.

This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.

f8ad232f1ebd19142135457de29d0b3156da9519

Issues

Description	From	Last Updated
'typing.Any' imported but unused Column: 1 Error code: F401	reviewbot	1 month, 3 weeks ago
While here, can you put these in parens?	chipx86	1 month ago
Module path is missing a ..	chipx86	1 month ago
Can we put that back in alphabetical order?	chipx86	1 month ago
For this case, I'm pretty sure a tuple is actually faster. A set doesn't buy us anything. Especially constructed every …	chipx86	1 month ago
While we're cleaning this up, can we pull out groups, inserts, removes, and differ into local variables, so we don't …	chipx86	1 month ago
Same note about the tuple vs. set.	chipx86	1 month ago
Same as above.	chipx86	1 month ago
Same as above.	chipx86	1 month ago
There are no open issues

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/opcode_generator.py (Diff revision 1)
The issue has been dropped. Show all issues
```
'typing.Any' imported but unused

Column: 1
Error code: F401
```

Commits:

	Summary	ID
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	946fe9dc79d2726d896130feeb7fcbd2faf57c7d
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	6961dc9470b3655bda576024c5e1c517818f1aa3

Diff:

Revision 2 (+818 -266)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/differ.py
	reviewboard/diffviewer/myersdiff.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/processors.py
	reviewboard/diffviewer/smdiff.py
	reviewboard/diffviewer/tests/test_myersdiff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	6961dc9470b3655bda576024c5e1c517818f1aa3
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f79f187458dcc42cf79371d2bc4670665d895ff8

Diff:

Revision 3 (+824 -266)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/differ.py
	reviewboard/diffviewer/myersdiff.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/processors.py
	reviewboard/diffviewer/smdiff.py
	reviewboard/diffviewer/tests/test_myersdiff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/myersdiff.py (Diff revision 3)
The issue has been resolved. Show all issues
```
While here, can you put these in parens?
```
reviewboard/diffviewer/opcode_generator.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Module path is missing a ..
```
reviewboard/diffviewer/opcode_generator.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can we put that back in alphabetical order?
```

reviewboard/diffviewer/opcode_generator.py (Diff revision 3)

The issue has been dropped. Show all issues

For this case, I'm pretty sure a tuple is actually faster. A set doesn't buy us anything. Especially constructed every time we do the check.

david 1 month ago

Ran a test on 4M checks, averaged across 20 runs. Sets are 13% faster. Both are extremely fast, though.

reviewboard/diffviewer/opcode_generator.py (Diff revision 3)

The issue has been resolved. Show all issues

While we're cleaning this up, can we pull out groups, inserts, removes, and differ into local variables, so we don't incur the lookup costs per-iteration?

reviewboard/diffviewer/opcode_generator.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same note about the tuple vs. set.
```
reviewboard/diffviewer/processors.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same as above.
```
reviewboard/diffviewer/processors.py (Diff revision 3)
The issue has been dropped. Show all issues
```
Same as above.
```

Commits:

	Summary	ID
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f79f187458dcc42cf79371d2bc4670665d895ff8
	Add typing for diff opcodes. This change updates diff opcodes to use explicitly typed tuples. The opcodes created by the `Differ` implementation have a tag and ranges, and then the opcode generator adds an additional metadata field. In the interest of avoiding any changes, we therefore have separate definitions for both steps. Testing Done: - Did a bunch of smoke testing of diff generation and saw that everything still worked correctly. - Ran unit tests.	f8ad232f1ebd19142135457de29d0b3156da9519

Diff:

Revision 4 (+838 -270)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/differ.py
	reviewboard/diffviewer/myersdiff.py
	reviewboard/diffviewer/opcode_generator.py
	reviewboard/diffviewer/processors.py
	reviewboard/diffviewer/smdiff.py
	reviewboard/diffviewer/tests/test_myersdiff.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (df24e42)