Create filename using bug numbers when downloading the diff

 






 

 


  Review Request #694
  —
  Created Jan. 13, 2009 and submitted

 


















 

  

   Information

  





 
 

  onkarshinde
 









 
 

  Review Board SVN (deprecated)
 









 
 

  
 









 
 

  815
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
The filename used while downloading the diff should be created using the bugs closed. This makes the file name unique for every diff and save the trouble of changing the name every time the diff is downloaded.

 








  


 
 

  

 







 









 





  








 









 






  

 








 














 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       /trunk/reviewboard/reviews/views.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Maybe do this if the filename is just "diff"? If someone uploads a diff, it will use that filename, and it may be more descriptive (including, say, a changeset number or date or something as an identifier). In that case, I'd hate to override it with something we generate.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   





 ON





   

   

  

 



 















 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       /trunk/reviewboard/reviews/views.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    It'd be nicer to store the generated filename in a 'filename' variable, and then set the 'Context-Disposition' to this after you've figured out what the filename is.

A couple issues:

1) bugs_closed may contain bugs separated by ", " instead of just ",". Whitespace should be taken into account. You should use review_request.get_bug_list(), which does all this.

2) If there's no bugs listed, this will be "bug.diff", which isn't very helpful. We'd need a better default.

3) Space after the comma in replace().

4) Make sure this is wrapped to less than 80 lines.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Forgive me, but isn't bugs-closed not necessarily unique? Multiple reviews may all address the same larger bug, and different revisions of diffs on the same review will definitely conflict.

Wouldn't <review request id>-<diff revision>.diff be a better format?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Probably. I wasn't completely comfortable with bug numbers either. Seems it would be useful in some installs, but something like the ID + revision, or somehow including the summary (though that could be pretty long) would be better.

It would also be nicer to have post-review come up with smarter filenames. But that's a separate thing.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      My reason for using bug numbers was to make it as unique as possible and easy to find when going through bunch of patches saved in a directory. Because I am more likely to remember a bug number when searching a patch than remembering review id.
I agree that having more than one review requests using same bug number will cause conflicts. Please let me know what approach should I follow in this case.

Meanwhile I am addressing Christian's comments.
      
 
      

      4. 



    4. 
 
 
      
 
      
  
 
      
 
      
  
      Christian and I discussed this, and I think both of us believe that <review id>-<diff id>.diff (or .patch) is the best option. Bug numbers are tempting, but given their non-uniqueness, especially with multiple diff revisions, I prefer something that maps more directly to what's in review board.

Admittedly, bug numbers may be easier to remember if you're looking at your bug list, but it's also not easy to get from patch -> review request with them, which I think is probably important if you're digging through a directory of patches.
      
 
      

      5. 



    5. 
 
 
      
 
      
  
 
      
 
      
  
      Nod. And the diff ID should be the revision, rather than the actual ID of the entry in the database.
      
 
      

      6. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   





 ON





   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  















  

   

    Diff:
   

   

    
 Revision 3 (+7 -4)
Show changes
 
    /trunk/reviewboard/reviews/views.py