• 
      

    Fix line counts returned by DiffCommitModel

    Review Request #7236 — Created April 22, 2015 and submitted

    Information

    Review Board
    dvcs
    6e8502a...

    Reviewers

    Previously, the total line counts returned was the concatenation of the
    keys of the lineCounts object. This has been fixed to return the
    actual sum of the elements in the object and the logic has been
    simplified. This fixes the visual style of the
    RB.DiffComplexityIconViews in the RB.DiffCommitIndexView.

    Verified the line counts returned by the getTotalLineCounts function
    were indeed an integer.

    Description From Last Updated

    Is the real difference between the two that we're now concatenating the values and not the keys? If so, we …

    chipx86 chipx86
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
      
      
      
      Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
      
      
    2. 
        
    chipx86
    1. 
        
    2. Show all issues

      Is the real difference between the two that we're now concatenating the values and not the keys? If so, we can we go back to the previous form of hte code?

      My reason being that, while there's a lot of neat functional stuff in underscore, it adds a readability overhead. Anyone coming in and working with this code will understand the loop and summing, but they'd have to look up and read through the docs of _.reduce.

      It's also a lot slower, because underscore doesn't chain, and is pretty bad with performance. So instead of one loop, we're doing three now: One to compute the values, one to compute the keys (internal to _.values, one to sum).

      If anything, given thow many line counts can be in a large file, we should probably go the other route and get rid of all function calls, just doing a native iteration of lineCounts.

      1. (Oops, failed at parens in that list of loops.. should have ended after "internal to _.values".)

    3. 
        
    brennie
    reviewbot
    1. Tool: PEP8 Style Checker
      Ignored Files:
          reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
      
      
      
      Tool: Pyflakes
      Ignored Files:
          reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    brennie
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to dvcs (1fd7044)