• 
      

    Clean up, document, and fix the organization of Review UIs.

    Review Request #9290 — Created Oct. 19, 2017 and submitted

    Information

    Review Board
    release-3.0.x
    67a460e...

    Reviewers

    Review UIs have received some modifications in this release (and in
    previous releases) that broke the separation of ReviewUI and
    FileAttachmentReviewUI. The former started receiving logic specific to
    file attachments, which belonged in the latter. It also provided a
    number of functions that made some assumptions about the types of
    objects it was working with, when it should have provided stub functions
    to overwrite.

    In practice, this isn't likely to have impacted extension authors, who
    are going to be subclassing FileAttachmentReviewUI, but non-attachment
    Review UIs would have required working around the misplaced logic. This
    change fixes all this so that the logic is all in the correct classes.

    It also adds a bunch of missing documentation, fixes bad documentation
    and error messages, implements necessary methods in
    LegacyScreenshotReviewUI, and fixes a regression with that UI's
    thumbnails that broke clipping of thumbnails.

    Unit tests pass.

    Manually tested thumbnails and review UIs for legacy screenshots, PDFs,
    images, and text files.

    Description From Last Updated

    This is totally opaque. I have no idea what "model class" means. Can you expand on this a bit?

    daviddavid

    Should probably be "an HTML thumbnail"

    daviddavid

    E501 line too long (93 > 79 characters)

    reviewbotreviewbot

    E501 line too long (93 > 79 characters)

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

    flake8

    david
    1. 
        
    2. reviewboard/reviews/ui/base.py (Diff revision 1)
       
       
      Show all issues

      This is totally opaque. I have no idea what "model class" means. Can you expand on this a bit?

      1. It's actually not used at all, so I'm getting rid of it.

    3. reviewboard/reviews/ui/base.py (Diff revision 1)
       
       
      Show all issues

      Should probably be "an HTML thumbnail"

    4. 
        
    chipx86
    Review request changed
    Change Summary:
    • Changed "a HTML" to "an HTML" in a docstring.
    • Got rid of ReviewUI.model, which was never used anywhere.
    Commit:
    4a887a533100eedd1746669c1424116e5f372694
    67a460ef0c2c177458731f6d3571e83137a70b8c

    Checks run (1 failed, 1 succeeded)

    flake8 failed.
    JSHint passed.

    flake8

    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.0.x (7730cb2)