Improve diff comment bubble tooltips to show rendered comment content

Review Request #6932 — Created Feb. 10, 2015 and submitted

Information

Review Board
master
f1b0a6b...

Reviewers

This is the first part in improving the diff comment bubbles.

The tooltip previews for the comment text will show the full rendered comment text together with the name of the reviewer, with the size of the tooltip maximized to the screen width.

Updated js tests as the comment reviewer needs to be loaded with each comment.

Ran tests, added markdown and plain text comments to check visually.

Added unit testing for XSS vulnerabilites. Manually checked by creating comments containing script tags for both markdown and plain text, scripts were escaped and not executed for both first load of the page and subsequent asynchronous updating of comments.


Description From Last Updated

Can we set a maximum width for this popup? The entire width of the page is a little excessive.

daviddavid

Can you go through and make sure that the styles of this match exactly what you see on the "View …

daviddavid

Col: 80 E501 line too long (89 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (93 > 79 characters)

reviewbotreviewbot

Col: 25 E131 continuation line unaligned for hanging indent

reviewbotreviewbot

This will allow non-Markdown text to inject tags into the page. Make sure you escape the text.

chipx86chipx86

Blank line between these. Though, you probably want RB.UserSession.instance instead.

chipx86chipx86

Can you make use of Underscore.js templates instead? That will help to keep all this more readable and maintainable.

chipx86chipx86

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

The opening ' characters should all line up, and indentation for the templates should happen within the string rather than …

daviddavid

What if the text is neither expected type nor SafeText?

ChesterChester

This should end in a period, and use the imperative mood ("Return" rather than "Returns").

daviddavid

Have seen the "insert in alphabetical order" comment before, perhaps applicable in this case.

SU Sunxperous

Same here for alphabetical order...

SU Sunxperous

A bit nitpicking, but is it better for row to be $row instead?

SU Sunxperous

I feel the name of the function doesn't clearly describe what this is for. Without the context of this being …

chipx86chipx86

"HTML".

chipx86chipx86

This isn't the right thing to pass to the comments in the serializedCommentBlocks. You should just be passing a dictionary, …

chipx86chipx86

This should stay a standalone function. If you're having a problem with this, then you can instead pull out this.tooltipTemplate …

chipx86chipx86

This can be one statement: list.append(tooltipTemplate({ ... }));

chipx86chipx86

Leftover debug code?

daviddavid

Can you rename list to be $list?

daviddavid

Because this is just one statement, it doesn't make a lot of sense to define a new function for it, …

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/resources/models/diffCommentModel.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/resources/models/diffCommentModel.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. reviewboard/reviews/context.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (89 > 79 characters)
    
  3. reviewboard/reviews/context.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (93 > 79 characters)
    
  4. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/resources/models/diffCommentModel.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/resources/models/diffCommentModel.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. reviewboard/reviews/context.py (Diff revision 2)
     
     
    Col: 25
     E131 continuation line unaligned for hanging indent
    
  3. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/resources/models/diffCommentModel.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/resources/models/diffCommentModel.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
david
  1. 
      
  2. Can we set a maximum width for this popup? The entire width of the page is a little excessive.

  3. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
chipx86
  1. Looking good.

    I want to make sure that the text is displayed just like it would be for reviews. I'd make use of the .rich-text CSS class for the tooltips, so that all styling is consistent.

    Also, please add testing (manual and unit tests) on XSS vulnerabilities. Neither rich text nor plain text should be able to inject any tags.

  2. reviewboard/reviews/context.py (Diff revision 5)
     
     
     

    This will allow non-Markdown text to inject tags into the page. Make sure you escape the text.

  3. Blank line between these.

    Though, you probably want RB.UserSession.instance instead.

  4. Can you make use of Underscore.js templates instead? That will help to keep all this more readable and maintainable.

  5. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. reviewboard/reviews/tests.py (Diff revision 7)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
david
  1. 
      
  2. reviewboard/static/rb/js/views/abstractCommentBlockView.js (Diff revision 8)
     
     
     
     
     
     
     
     
     

    The opening ' characters should all line up, and indentation for the templates should happen within the string rather than outside of it.

  3. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
david
  1. Would you mind uploading a screenshot that compares what is shown in the tooltip to what is shown on the reviews page?

    1. I've added the reviews page screenshot, you can compare it with the first screenshot for markdown comments.

  2. 
      
WU
david
  1. 
      
  2. Can you go through and make sure that the styles of this match exactly what you see on the "View Reviews" page? Notably, all text should be the same size, and all displayed in a fixed-width font.

    1. I've updated the screenshots for the new styling. (for some reason the old markdown comments.png disappeared when I uploaded and hit discard changes earlier)

      One thing to note is that I've changed the html tag to pre, all line breaks are preserved so the tooltip can get very long.

  3. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
david
  1. I only see one trivial thing left.

  2. reviewboard/reviews/markdown_utils.py (Diff revision 10)
     
     

    This should end in a period, and use the imperative mood ("Return" rather than "Returns").

    1. Using imperative mood will make it inconsistent with the rest of the functions.

    2. We're working on moving toward this for all functions, so that's fine.

  3. 
      
Chester
  1. 
      
  2. reviewboard/reviews/tests.py (Diff revision 9)
     
     

    What if the text is neither expected type nor SafeText?

    1. I'm not sure I get your question. The text is meant to be displayed as html thus it will contain html tags, making it not SafeText, even plain text will have <p> tags after being rendered as markdown.

      What do you mean by expected type?

    2. I agree, it seems like the check is fine as you have it.

  3. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
SU
  1. 
      
  2. reviewboard/reviews/context.py (Diff revision 11)
     
     

    Have seen the "insert in alphabetical order" comment before, perhaps applicable in this case.

  3. reviewboard/reviews/tests.py (Diff revision 11)
     
     

    Same here for alphabetical order...

  4. Just curious, should this not be placed in beforeEach, instead of being repeated three times for each it block?

    1. You're right, I've changed it.

  5. A bit nitpicking, but is it better for row to be $row instead?

  6. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
chipx86
  1. 
      
    1. Also, I forgot to mention. It looks like this will only do the right thing for diffs. This also needs to work for file attachments (text-based files and image files). You'll want to look for the comment serialization in reviews/ui/base.py.

  2. reviewboard/reviews/markdown_utils.py (Diff revision 12)
     
     

    I feel the name of the function doesn't clearly describe what this is for. Without the context of this being in Markdown-related code, it's confusing what this is for.

    How about markdown_render_conditional?

  3. reviewboard/reviews/markdown_utils.py (Diff revision 12)
     
     

    "HTML".

  4. This isn't the right thing to pass to the comments in the serializedCommentBlocks. You should just be passing a dictionary, just like what's passed to the page. a UserSession is a very different thing.

  5. This should stay a standalone function. If you're having a problem with this, then you can instead pull out this.tooltipTemplate into a variable in _updateTooltip.

  6. reviewboard/static/rb/js/views/abstractCommentBlockView.js (Diff revision 12)
     
     
     
     
     
     
     

    This can be one statement:

    list.append(tooltipTemplate({
        ...
    }));
    
  7. 
      
WU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/ui/base.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/ui/base.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/ui/base.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/ui/base.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
david
  1. Just a few simple comments left.

  2. reviewboard/reviews/ui/base.py (Diff revision 14)
     
     

    Leftover debug code?

  3. Can you rename list to be $list?

  4. reviewboard/static/rb/js/views/abstractCommentBlockView.js (Diff revision 14)
     
     
     
     
     
     
     
     

    Because this is just one statement, it doesn't make a lot of sense to define a new function for it, even with the small amount of duplication below.

    We should also not be calling gettext() on the username.

  5. 
      
WU
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/ui/base.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/markdown_utils.py
        reviewboard/reviews/tests.py
        reviewboard/reviews/ui/base.py
        reviewboard/reviews/context.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js
        reviewboard/static/rb/css/pages/search.less
        reviewboard/static/rb/js/models/abstractCommentBlockModel.js
        reviewboard/static/rb/js/resources/models/baseCommentModel.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/static/rb/js/models/commentEditorModel.js
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
WU
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (5d758f0)
Loading...