Add line numbers to reviews in text emails.

Review Request #6864 — Created Feb. 1, 2015 and submitted

Information

Review Board
master
f3babef...

Reviewers

When a review has comments on the source code, text email notifications only show the files that have been changed, while HTML email notifications show the diff view. This change adds the line numbers of the patched files that have been commented on in a review to the email text body.

Now, the line numbers are shown as follows:

<file name> (Lines 18 - 25)

A new template tag has been implemented to render the line numbers of the code that have been commented on. It will render singular "Line" or plural "Lines" or none depending on the changes.

In reviewboard/reviews/views.py, we have added and exposed the chunks attribute to each comment entry within comment_entries. Also, the text email template will now make use of this new exposed attribute to render the individual commented files with line number information in each review.

While testing this, we found a new bug:
https://code.google.com/p/reviewboard/issues/detail?id=3748

No unit tests have been written.

Manual testing has been conducted:
When no lines are selected in a review, nothing is affected.
When a range of lines over multiple chunks are selected, the line numbers display properly.
Chunks which have been deleted will not have their original line numbers shown, only line numbers in the patched file will be shown.

Description From Last Updated

Col: 1 W391 blank line at end of file

reviewbotreviewbot

Can you combine these? """Renders ...

daviddavid

Remove this blank line.

daviddavid

Can we wrap the format strings in gettext? We already import it, so you can just do _('(Line %d)')

daviddavid

We probably need to define an empty 'chunks' list here, in case there was an exception in get_line_counts() or get_file_chunks_in_range()

daviddavid

Blank line between a statement and a new block

brenniebrennie

Has the error template changed to require the chunks parameter?

brenniebrennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
  2. Col: 1
     W391 blank line at end of file
    
  3. 
      
SU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
  2. 
      
david
  1. 
      
  2. Can you combine these? """Renders ...

  3. Remove this blank line.

  4. reviewboard/reviews/templatetags/reviewtags.py (Diff revision 2)
     
     
     
     

    Can we wrap the format strings in gettext? We already import it, so you can just do _('(Line %d)')

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

    We probably need to define an empty 'chunks' list here, in case there was an exception in get_line_counts() or get_file_chunks_in_range()

  6. 
      
SU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
  2. 
      
brennie
  1. 
      
  2. Blank line between a statement and a new block

  3. reviewboard/reviews/views.py (Diff revision 3)
     
     

    Has the error template changed to require the chunks parameter?

    1. No, the error template does not require the chunks parameter. Neither does it contain the domain and domain_method parameter, but those are still passed into the renderer. Should they be removed?

  4. 
      
SU
SU
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/templates/notifications/review_email.txt
    
    
  2. 
      
TE
  1. Looks good to me!

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

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (c396156)
Loading...