• 
      

    Added an issue summary to a review thread.

    Review Request #2617 — Created Sept. 24, 2011 and submitted

    Information

    Review Board

    Reviewers

    Added summary table HTML code within the review_detail template, along with relevant CSS in reviews.css.
    
    Add an issues variable to the review_detail view response for direct access to issue statuses and counts.
        
    Added get_all_ccoments function to the Review model in the reviews models.py in order to traverse through the comments once within the template.
        
    Added pretty_print_status and char_limit filters to use within template.
    
     
    Description From Last Updated

    Make sure you always have a space before the {

    daviddavid

    You have another ".issue-table td" block on line 1204.

    daviddavid

    You should be able to move these two rules into ".issue table td" block below, and avoid repeating them in …

    daviddavid

    Wrap this to 80 columns. You can wrap without ugly \ characters by putting everything in parantheses: return (list(self.comments.all()) + …

    daviddavid

    For single-line docstrings, you can do it all on one line: """Turns an issue status code into a human-readable status …

    daviddavid

    Same here re: docstring.

    daviddavid

    Remove this extra line

    daviddavid

    This looks like it has tab characters. Make sure you run pep8

    daviddavid

    You can simplify this quite a bit using the static methods in BaseComment: issues['total'] += 1 issues[BaseComment.issue_status_to_string( comment.issue_status)] += 1

    daviddavid

    Don't include spaces between the {{s and the variable names

    daviddavid

    Same here re: spaces

    daviddavid

    Can you call this "pretty_print_issue_status" instead? The name "pretty_print_status" is very generic.

    daviddavid

    Need a space between td and {

    daviddavid
    david
    1. 
        
    2. Show all issues
      Make sure you always have a space before the {
    3. reviewboard/htdocs/media/rb/css/reviews.css (Diff revision 1)
       
       
       
      Show all issues
      You should be able to move these two rules into ".issue table td" block below, and avoid repeating them in each of the states.
    4. reviewboard/reviews/models.py (Diff revision 1)
       
       
      Show all issues
      Wrap this to 80 columns. You can wrap without ugly \ characters by putting everything in parantheses:
      
      
      return (list(self.comments.all()) +
              list(self.screenshot_comments.all()) +
              list(self.file_attachment_comments.all()))
      
    5. reviewboard/reviews/templatetags/reviewtags.py (Diff revision 1)
       
       
       
       
      Show all issues
      For single-line docstrings, you can do it all on one line:
      
      """Turns an issue status code into a human-readable status string."""
    6. reviewboard/reviews/templatetags/reviewtags.py (Diff revision 1)
       
       
       
       
      Show all issues
      Same here re: docstring.
    7. Show all issues
      Remove this extra line
    8. reviewboard/reviews/views.py (Diff revision 1)
       
       
      Show all issues
      This looks like it has tab characters. Make sure you run pep8
    9. reviewboard/reviews/views.py (Diff revision 1)
       
       
       
       
       
       
       
       
       
      Show all issues
      You can simplify this quite a bit using the static methods in BaseComment:
      
      issues['total'] += 1
      issues[BaseComment.issue_status_to_string(
          comment.issue_status)] += 1
    10. Show all issues
      Don't include spaces between the {{s and the variable names
    11. Show all issues
      Same here re: spaces
    12. 
        
    ME
    ME
    david
    1. Getting pretty close now.
    2. reviewboard/htdocs/media/rb/css/reviews.css (Diff revisions 1 - 2)
       
       
       
       
       
       
      Show all issues
      You have another ".issue-table td" block on line 1204.
    3. Show all issues
      Can you call this "pretty_print_issue_status" instead? The name "pretty_print_status" is very generic.
    4. 
        
    ME
    ME
    david
    1. Just one trivial change left.
    2. Show all issues
      Need a space between td and {
    3. 
        
    ME
    ME
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (4397df7)