• 
      

    Backport the new interdiff filtering algorithm and make it optional.

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

    Information

    Review Board
    release-3.0.x

    Reviewers

    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 ID
    Backport the new interdiff filtering algorithm and make it optional.
    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: ```python 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.
    7fd4e37261aaeb9afcf2c07f21dabf604bb371a7
    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
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.0.x (c9113ef)