• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    Support choices with differing required values in a ConditionSet.
    
 
    

    



 
    
 
    

  Review Request #8349
  —
  Created Aug. 25, 2016 and submitted 

 
    


    




    




    




 
    
  
    
   Information

  
    



    
 
 
    
  chipx86
 
    

    





    
 
 
    
  Djblets
 
    

    





    
 
 
    
  release-0.10.x
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  364629b...
 
    

    



 
    



 
    
  
    
   Reviewers

  
    



    
 
 
    
  djblets
 
    

    





    
 
 
    
  
 
    

    



 
    



    


    
 
    




  

    
 
 
    
  
    Previously, a ConditionSet's list of choices all had to accept the same
    
exact type of value for matches. Callers couldn't mix-and-match
    
different types of condition choices. If there were two pieces of data
    
to work with (say, a book and an author), the list of choices all had to
    
accept one of those, and infer the other (if possible).
    

    Now, ConditionSet.matches() takes keyword arguments for the values,
    
instead of a single positional argument for a single value. These are
    
compared to BaseConditionChoice.value_kwarg for each condition being
    
matched.
    

    By default, choices expect a value= keyword argument. More advanced
    
usage might have some choices that take book= and others that take
    
author=, as per the example above.
    
 
    

    





  

    
 
 
    
  
    Unit tests pass.
    
 
    

    




 
    

    





    
 
    









 





    
 
 
 
    
  
    
   
    
    
    
     
    
    
    
      
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
     
    • 
      
     
      • 
    
    
   
    
   
    
    
     
    
      Description
      From
      Last Updated
     
    
        
    



     
    
      
       
       
    This whole thing could be reduced to: results = list(results) return results and all(results)
    
          		
      
       daviddavid
      
      
       
      
     
    

        
   
    
  
    
 
    

    




 
    

    



  
    
 
    

    


    


 













    
 





 
    
  
    
   
    



reviewbot




   
    
   
    



    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
reviewbot


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Tool: Pyflakes
Processed Files:
    djblets/conditions/conditions.py
    djblets/conditions/choices.py
    djblets/conditions/tests/test_conditions.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/conditions/conditions.py
    djblets/conditions/choices.py
    djblets/conditions/tests/test_conditions.py


      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 















    2. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 


    

  
    
 
    

    












    
 





 
    
  
    
   
    



david




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 

















    2. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       djblets/conditions/conditions.py
       


        (Diff revision 1)


       
      
     
      
          
    
     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   Show all issues
  
 
      

      



    
      This whole thing could be reduced to:
      

      

      results = list(results)
return results and all(results)

      
  
      
 
      

 
      
  


      
 
      
 
        

  


      1. 
 
 
        
 
        
  
 
        
 
        
  
        It could, but I'm intentionally avoiding computing all of them up-front.
        

        Condition sets are not always going to match, which is basically the point. A condition set might be designed to notify a particular Slack channel based on repository and diff file path. Processing these lazily, using the generator, means that if one condition (say, the repository match) fails early, we don't have to process other potentially expensive conditions (like computing diff paths, or something else).
        

        If we fetch it all up-front, we process them all, likely unnecessarily.
        

        That's why it's doing a more unrolled version of that check. So we can check the bare minimum but still compute whether we processed any results.
        
 
        

        2. 


 
      


      

 
      

      3. 




    3. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      4. 


    

  
    
 
    

    












    
 





 
    
  
    
   
    



david




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Ship It!
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 















    2. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



chipx86




   
    
   
    
  
    
 
    


 
    
  
    

   
    
   
    
    
     Review request changed
    
    
   
    

  
    
  
    
  
    




    

  
    
   
    
    Status:
   
    
   
    

    Completed

   
    
  
    



  
    
   
    
    Change Summary:
   
    
   
    
    
    Pushed to release-0.10.x (6368403)