FIx for issue 121. svn cp/mv not handled well by review board.

 






 

 


  Review Request #367
  —
  Created April 26, 2008 and submitted

 


















 

  

   Information

  





 
 

  jeeva
 









 
 

  Review Board SVN (deprecated)
 









 
 

  
 









 
 

  121
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This is primarily a patch for http://code.google.com/p/reviewboard/issues/detail?id=121 - along with some relevant refactorings.

Changes include
 * Adding function handle_renames which acts like a filter to fix the svn mv/cp issue.
 * Changed convert_absolute_paths to return a list of results like handle_renames for consistency.
 * Made convert_absolute_paths ignore files which where already had an absolute path
 * added a function svn_info which converts the results of 'svn info filename' to a dict and refactored convert_absolute_paths to use it.

 








  


 
 

  
I hacked this together because I had a changeset which did do a lot of svn mv/cp. So thats how I tested these changes.

I also checked that it still worked on vanilla changes without svn mv/cp.

 







 









 





  








 









 






  

 








 














 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david



    
   


  

  

  



    




  1. 
 
    
  
    
   
    This is mostly just nitpicking.  The implementation itself looks great.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Nitpicking is good. Its all too easy to miss the little stuff :)
      
 
      

      2. 


 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       /trunk/reviewboard/contrib/tools/post-review
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    It would be nice to give a little bit more detail here.  The problem is really that the diff contains parent revisions for the "new" files.

Comments should also be properly sentence cased (start with a capital letter) and end with a period.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Done - although it's a bit wordy (its late here in Australia...)
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       /trunk/reviewboard/contrib/tools/post-review
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Should be a space between the # and the comment.  Also should be capitalized/punctuated.
    
  
    
 
    

 
    
  



 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       /trunk/reviewboard/contrib/tools/post-review
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Trailing whitespace.
    
  
    
 
    

 
    
  



 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       /trunk/reviewboard/contrib/tools/post-review
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Space between # and We.  This should also be wrapped to 80 characters.
    
  
    
 
    

 
    
  



 
    

    6. 




  6. 
 
    
  
    
   
    This should also fix bug 288, right?
    
  
    
 
    

 
    
  


    
 
    
 
      

  







 
    


    

 
    

    7.