Backport the new interdiff filtering algorithm and make it optional.

Review Request #11051 — Created June 22, 2020 and updated

chipx86
Review Board
release-3.0.x
reviewboard

A long-standing complaint in Review Board 3.0 are the edge cases that
can sometimes be hit with the interdiff filtering algorithm. This has
been difficult to solve, and a great deal of time and investigation has
gone into this for Review Board 4.0. We believe we've improved upon the
algorithm enough to cover all these edge cases, but we need testing.

This change backports the new code to 3.0, making it optional. The new
logic is disabled by default, as we don't want to regress any installs,
but administrators can enable it by setting the following in the
settings_local.py file:

ENABLED_FEATURES = {
    'diffviewer.filter_interdiffs_v2': True,
}

and then either viewing a new interdiff or clearing memcached and
viewing an existing one.

This mechanism allows the new algorithm to be easily turned off if it's
causing trouble.

We're hoping to get people who reported issues in the past to test this
in 3.0.18+, so we can be sure that 4.0 users will have a fully working
interdiff algorithm.

Much of the core algorithmic work was reviewed previously in
https://reviews.reviewboard.org/r/8834/ and commited in cc6a91fa. This
code focuses on bringing in the feature flag support, funneling in the
request object to filter_interdiff_opcodes(), and keying off that
logic by flag.

This is not the final version of this algorithm. Work has been underway
on further improving it to improve upon the output and to add a better
test harness for real-world anonymized diffs, in order to help us build
up a better regression test suite.

Unit tests pass.

Manually tested my interdiff regressions collection with and without
the new algorithm enabled.

Summary
Backport the new interdiff filtering algorithm and make it optional.
Description From Last Updated

E302 expected 2 blank lines, found 1

reviewbotreviewbot

F401 'json' imported but unused

reviewbotreviewbot

F401 'os' imported but unused

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
Review request changed

Change Summary:

  • Fixed a missing blank line.
  • Removed unused imports.

Commits:

Summary
-
Backport the new interdiff filtering algorithm and make it optional.
+
Backport the new interdiff filtering algorithm and make it optional.

Diff:

Revision 2 (+1838 -16)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
david
  1. Ship It!
  2. 
      
Loading...