• 
      

    Move the main diffviewer views to extensible class-based views.

    Review Request #4471 — Created Aug. 21, 2013 and submitted

    Information

    Review Board
    master

    Reviewers

    Move the main diffviewer views to extensible class-based views.
    
    Django's pushing for class-based generic views for things, and in the
    case of the diff viewer, it provides a lot of advantages.
    
    So far, diffviewer has had a nice separation, where the code in
    diffviewer was able to focus solely on diff-related work, and the code
    in reviews was able to focus on objects related to review requests. This
    was getting hard to maintain with the new review UI support in the diff
    viewer, though, and it was clear a more extensible solution was needed.
    
    Now, the code for rendering the diff viewer and diff fragments are
    class-based. This simplifies part of the code, and gives us a foundation
    for some better extensibility by subclasses. The code in reviews will
    soon be able to do a lot more with the resulting renders without having
    to inject a bunch of new code into diffviewer.
    
    No logic was changed, but much of the code moved around.
    
    Going forward, some mixins for things like fetching review request data
    and checking login support will be created to reduce what these classes
    have to do. I'm leaving them out of this change, though.
    Tested standard diffs, interdiffs, and selecting revisions.
    
    Unit tests pass.
    Description From Last Updated

    local variable 'context' is assigned to but never used

    reviewbotreviewbot

    Col: 20 E127 continuation line over-indented for visual indent

    reviewbotreviewbot

    Col: 9 E123 closing bracket does not match indentation of opening bracket's line

    reviewbotreviewbot

    There's not a lot of point to having this be a unicode string--it doesn't contain any non-ascii characters, and _() …

    daviddavid

    Col: 9 E123 closing bracket does not match indentation of opening bracket's line

    reviewbotreviewbot

    Col: 9 E123 closing bracket does not match indentation of opening bracket's line

    reviewbotreviewbot

    Col: 9 E123 closing bracket does not match indentation of opening bracket's line

    reviewbotreviewbot
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. reviewboard/diffviewer/views.py (Diff revision 1)
       
       
      Show all issues
      Col: 20
       E127 continuation line over-indented for visual indent
      
    3. reviewboard/reviews/views.py (Diff revision 1)
       
       
      Show all issues
      Col: 9
       E123 closing bracket does not match indentation of opening bracket's line
      
    4. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. reviewboard/diffviewer/views.py (Diff revision 1)
       
       
      Show all issues
       local variable 'context' is assigned to but never used
      
    3. 
        
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. reviewboard/reviews/views.py (Diff revision 2)
       
       
      Show all issues
      Col: 9
       E123 closing bracket does not match indentation of opening bracket's line
      
    3. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. 
        
    david
    1. Most of this looks great. I'd like to see more docstrings.
    2. reviewboard/diffviewer/views.py (Diff revision 2)
       
       
       
      Show all issues
      There's not a lot of point to having this be a unicode string--it doesn't contain any non-ascii characters, and _() will return unicode.
    3. 
        
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. reviewboard/reviews/views.py (Diff revision 3)
       
       
      Show all issues
      Col: 9
       E123 closing bracket does not match indentation of opening bracket's line
      
    3. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. 
        
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. reviewboard/reviews/views.py (Diff revision 4)
       
       
      Show all issues
      Col: 9
       E123 closing bracket does not match indentation of opening bracket's line
      
    3. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/reviews/views.py
          reviewboard/diffviewer/views.py
          reviewboard/reviews/urls.py
        Ignored Files:
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed