Deprecate the type argument of review request signals

 






 

 


  Review Request #8896
  —
  Created April 12, 2017 and submitted 

 


















 

  

   Information

  





 
 

  brennie
 









 
 

  Review Board
 









 
 

  release-3.0.x
 









 
 

  
 









 
 

  8895
 









 
 

  8897
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This patch deprecates the type argument of the review_request_closed

and review_request_closing signals, due to shadowing the type

builtin. We still provide the type argument, but it is wrapped in a

lazy object that will emit a warning when casted to a unicode string. We

now include the close_type argument which takes the place of the old

type argument. Usages and unit tests have been updated for this new

behaviour.

 








  


 
 

  
Ran unit tests.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Typo in summary: rquest

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
this file isnt supposed to be in the diff.

      		
      
       brenniebrennie
      
      
       
      
     




     

      
       
       
Sooo the difficulty here is that if an extension does: review_request.close(type=..., ...) then things are going to break. The problem …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Can you pass these as keyword arguments? It'll help with readability a lot.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Here, too.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
This should have a .. deprecated::

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Here, too.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
This would be great in Djblets :)

      		
      
       chipx86chipx86
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 



 

  

   




brennie




   

   

  

 



 















 





 

  

   




brennie




   

   







  

 



 

  


   

   

    
 
brennie


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    this file isnt supposed to be in the diff.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




brennie




   

   

  

 



 















 





 

  

   




david




   

   









  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Typo in summary: rquest
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 













  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




brennie




   

   

  

 



 















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Sooo the difficulty here is that if an extension does:
    

    

    review_request.close(type=..., ...)

    

    

    then things are going to break. The problem is that if **kwargs is provided, and we try to check for it there, then it'll still break since close_type won't be supported.
    

    Hmm..
    

    Not the end of the world, but would be nice to fix.
    

    Perhaps we could say that close() will, by default, close as submitted. Then we can do that and have a **kwargs that checks for type and overrides close_type?
    

    I think submitted is going to be far more common than discarded, so I wouldn't have a problem with this as a default.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Can you pass these as keyword arguments? It'll help with readability a lot.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/reviews/models/review_request.py
      


       (Diff revision 3)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Here, too.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/reviews/signals.py
      


       (Diff revision 3)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This should have a .. deprecated::
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 






  6. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/reviews/signals.py
      


       (Diff revision 3)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Here, too.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 






  7. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/signals.py
      


       (Diff revision 3)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This would be great in Djblets :)
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Can we put this into another change later? I really wanna get this in.
      
 
      

      2. 


 
    


    

 
    

    8. 




  8. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    9. 




  

 
















 



 

  

   




brennie




   

   

  

 



 















 



 

  

   




brennie




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to release-3.0.x (9a02c1e)





 


 










  

 









     

    

   

  

  

   
   Loading...