Review Board 1.5.5

Ticket #419 - Stack trace when downloading Subversion Export File complains about no method "bytesize"

Updated 2 years, 3 months ago

Mike Conley Reviewers
branches/release_0.5 markus_developers
None MarkUs Source Code Repository 
get_svn_commands was returning an Array, not a String.  Plus, get_svn_commands didn't take into account the possibility that there would be spaces in Group names.
No regressions with units/functionals.

Wrote 2 new test cases in Assignment_Test - though I'm concerned that I essentially recreated the get_svn_commands method inside the test, just to generate what I'm comparing against... 

Maybe I just need some sleep, but are there any ideas on a better way of doing this?
Mike Conley
Posted 2 years, 3 months ago (October 2nd, 2009, 3:10 p.m.)

   

  

   
    
    
   


  




   



    

     
     
    

    

     
See what I mean, about reinventing?

     

  

   
    
    
   


  



    

    

   


  
 




  

 
















 

  




 

 

  
  
Severin Gehwolf

  
Posted 2 years, 3 months ago (October 2nd, 2009, 6:01 p.m.)

 

 

   
Mike, you are pretty busy, eh? BTW. that was something I coded. Thanks for the cleanup!
More bugs to fix for 0.5? Anything I can help with?

   

  

   
    
    
   


  




   



    

     
     
    

    

     
Idea:

Why bother if there are spaces in there or not? How about this?

svn_commands.push("svn export -r #{submission.revision_number} #{grouping.group.repository_external_access_url} \"#{grouping.group.group_name}\"")

     

  

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    You're right, I like that better.  I'll upload an updated diff soon.
    
    
    
   
    2. 

   


  



    

    

    

     
     
    

    

     
I wander what assert_equal does for Array instances. Does it compare references? I think, what we should do in those cases is actually loop over the array and to assertions on the array elements.

     

  

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    From the Ruby documentation:

Two arrays are equal if they contain the same number of elements and if each element is equal to (according to Object.==) the corresponding element in the other array. 

So I think it compares by value.
    
    
    
   
    2. 

   


  



    

    

    

     
     
    

    

     
What if we would loop over the array returned by get_svn_commands() here and do a assert_match(/\"/, svn_command)?


     

  

   
    
    
   


  



    

    

   


  
 




  

 
















 

  




 

 

  
  
Severin Gehwolf

  
Posted 2 years, 3 months ago (October 2nd, 2009, 6:21 p.m.)

 

 

   

   

  

   
    
    
   


  




   



    

     
     
    

    

     
Totally my bad.

     

  

   
    
    
   


  



    

    

   


  
 




  

 














 

  




 

   Review request changed
   
Updated 2 years, 3 months ago (October 4th, 2009, 6:33 p.m.)

 

 

  

  
  
Went with Severin's idea, and changed the SVN Export output so that group names with spaces are in quotes instead of underscores.


 




  

 















 

  




 

 

  
Ship it!

  
Severin Gehwolf

  
Posted 2 years, 3 months ago (October 4th, 2009, 7:41 p.m.)

 

 

   
Awesome job! Nothing to add from my side.