• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    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. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       reviewboard/notifications/email/decorators.py
       


        (Diff revision 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. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       reviewboard/reviews/models/review_request.py
       


        (Diff revision 3)


       
      
     
      
          
    
     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   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. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       reviewboard/reviews/models/review_request.py
       


        (Diff revision 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:
   
    
   
    

    Completed

   
    
  
    



  
    
   
    
    Change Summary:
   
    
   
    
    
    Pushed to release-3.0.x (9a02c1e)