• 
      

    Make "Update Diff" work like "New Review Request"

    Review Request #6138 — Created July 23, 2014 and submitted

    Information

    Review Board
    master
    88b100f...

    Reviewers

    This change replaces the old "Update Diff" dialog with a new version that's
    based on the pre-commit workflow from the "New Review Request" page. This makes
    it so that the general case is as simple as selecting or drag-and-dropping a
    diff file, and other questions (such as the basedir or parent diff) are only
    asked if necessary.

    The PreCommitView from the New Review Request page has been generalized into
    UploadDiffView, which both the PreCommitView and UpdateDiffView extend. For the
    most part, the only difference between these are the specific templates that
    are used, and the fact that the update dialog uses $.fn.modalDlg around
    this.$el.

    This eliminates one of our two remaining uses of $.formDlg and gReviewRequest,
    getting us that much closer to a clean javascript codebase.

    • Went through all the old scenarios with the "New Review Request" page to test
      for regressions.
    • Updated a diff on a review request and saw the resulting change.
    • Updated a diff on a review request that required a parent diff, specified
      that parent diff, and saw the resulting change.
    • Tried to update a diff on a review request with a patch file that had short
      revs for git and saw the appropriate error.
    • Updated a diff on a review request using an svn repository and was prompted
      for the base directory.

    Description From Last Updated

    Space after switch.

    chipx86chipx86

    Should have a break; after, just to keep issues from popping up down the road.

    chipx86chipx86

    <tt> is more appropriate than <code> here.

    chipx86chipx86

    We should move this file if it's no longer specific to the New Review Request page.

    chipx86chipx86

    Can these go in alphabetical order?

    chipx86chipx86
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/staticbundles.py
      
      Ignored Files:
          reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js
          reviewboard/templates/reviews/reviewable_page_data.js
          reviewboard/static/rb/js/views/reviewRequestEditorView.js
          reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
          reviewboard/static/rb/js/views/uploadDiffView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/static/rb/js/views/updateDiffView.js
          reviewboard/templates/reviews/review_request_dlgs.html
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/staticbundles.py
      
      Ignored Files:
          reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js
          reviewboard/templates/reviews/reviewable_page_data.js
          reviewboard/static/rb/js/views/reviewRequestEditorView.js
          reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
          reviewboard/static/rb/js/views/uploadDiffView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/static/rb/js/views/updateDiffView.js
          reviewboard/templates/reviews/review_request_dlgs.html
      
      
    2. 
        
    chipx86
    1. Nice :)

    2. Show all issues

      Space after switch.

    3. Show all issues

      Should have a break; after, just to keep issues from popping up down the road.

    4. Show all issues

      <tt> is more appropriate than <code> here.

    5. reviewboard/staticbundles.py (Diff revision 1)
       
       
      Show all issues

      We should move this file if it's no longer specific to the New Review Request page.

    6. reviewboard/staticbundles.py (Diff revision 1)
       
       
       
      Show all issues

      Can these go in alphabetical order?

      1. Nope, it's dependency order.

    7. 
        
    david
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/staticbundles.py
      
      Ignored Files:
          reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
          reviewboard/static/rb/js/views/updateDiffView.js
          reviewboard/static/rb/js/models/uploadDiffModel.js
          reviewboard/templates/reviews/reviewable_page_data.js
          reviewboard/static/rb/js/views/reviewRequestEditorView.js
          reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
          reviewboard/static/rb/js/views/uploadDiffView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/review_request_dlgs.html
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/staticbundles.py
      
      Ignored Files:
          reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
          reviewboard/static/rb/js/views/updateDiffView.js
          reviewboard/static/rb/js/models/uploadDiffModel.js
          reviewboard/templates/reviews/reviewable_page_data.js
          reviewboard/static/rb/js/views/reviewRequestEditorView.js
          reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js
          reviewboard/static/rb/js/views/uploadDiffView.js
          reviewboard/static/rb/css/pages/reviews.less
          reviewboard/templates/reviews/review_request_dlgs.html
      
      
    2. 
        
    chipx86
    1. Ship It!

    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (802be4a)