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

Diff:

Revision 2 (+569 -140)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (7730cb2)
Loading...