Make six use consistant.




 


  Review Request #9649
  —
  Created Feb. 17, 2018 and submitted 

 















 
  
  
 




 
  

   Information

  

 
 

  

   
   
solarmist


  


  

   
   
RBTools


  


  

   
   
master


  


  

   
   



  


  

   
   
9648


  


  

   
   
9651, 9650


  


  

   
   
8a990da...


  


 



 
  

   Reviewers

  

 
 

  

   
   
rbtools


  


  

   
   



  


 











 

  
  


Make six use consistant.


Import the function directly if it is a six.moves function otherwise

leave it specified as a six function that needs to be replaced in

the future.




 




 

  
  


All tests passed in python 2.7




 














 

 






 
 
 

  

   

    
   

   
    
    
  • 
     
    
    • 
    
  • 
     
    
    • 
    
  • 
     
    
    • 
    
  • 
     
    
    • 
    
  • 
     
    
    • 
   

  

  

   
    

     Description
     From
     Last Updated
    

   
   



    

     
      
      
We don't want to only import six. That makes updating code down the road much harder, increases the chances of …

     		
     
      chipx86chipx86
     
     
      
     
    




    

     
      
      
Please undo this change. The previous way was better.

     		
     
      daviddavid
     
     
      
     
    




    

     
      
      
Please undo these changes.

     		
     
      daviddavid
     
     
      
     
    




    

     
      
      
F821 undefined name 'six'

     		
     
      reviewbotreviewbot
     
     
      
     
    




    

     
      
      
This is changing StringIO from using the C implementation to using the Python implementation (on Python 2). Please change this …

     		
     
      daviddavid
     
     
      
     
    


   
  

 












  

 



















 




 




 














 



 

  

   




solarmist




   

   

  

 



 















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    We don't want to only import six. That makes updating code down the road much harder, increases the chances of errors (we can't check for unimported references anymore), and just generally makes things more verbose. I don't really understand the reason for this change, but it's not the direction we want to take the codebase.
    

    This change is also trying to do too much. Some parts are changing how six imports work, others are moving things like unicode() and .iteritems() to six equivalents. That should be its own change.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      The idea is that the six library is a transitional library only as all references to six should be removed as soon as Python 2.7 support is dropped.  This makes it an easy task to find all such references and remove them.  As you said yourself in another review it makes them easy to grep.
      

      That said, only a handleful of the fixes are required.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      It should be, yes, but this change makes it harder to drop six. With the way we do six imports, we're typically using the name naming for classes/functions, and dropping Python 2.7 means just replacing the imports for most cases. This change makes that harder, since we now have to replace a lot more lines of code a lot more carefully, with less tool assistance.
      
 
      

      3. 


 
    


    

 
    

    3. 













  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




solarmist




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/clients/tests/test_svn.py
      


       (Diff revision 2)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Please undo this change. The previous way was better.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/utils/testbase.py
      


       (Diff revision 2)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Please undo these changes.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 



 

  

   




solarmist




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  











 



  

   
Commit:

   
    
-cff3904fb199a79c80cb6b39c1b90642d6e83bc8
+d9c583274b6654f9ed42f19928957858f76da44a

   		
  




  

   
Diff:

   
    
 Revision 3 (+6 -4)
Show changes
 
    rbtools/api/tests.py 
    rbtools/clients/svn.py 
    rbtools/clients/tests/test_mercurial.py 
    rbtools/clients/tests/test_svn.py 
    rbtools/utils/testbase.py 
 

   		
  



 









 

  
Checks run (1 failed, 1 succeeded)



  

 flake8
 failed.
 





  

 JSHint
 passed.
 





 







 
flake8

 


 















  

 
















 



 

  

   




solarmist




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/clients/tests/test_svn.py
      


       (Diff revision 4)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    This is changing StringIO from using the C implementation to using the Python implementation (on Python 2). Please change this back.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




solarmist




   

   

  

 



 















 



 

  

   




solarmist




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




solarmist




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to master (fcb7b90)





 


 










  

 









     

    

   

  

  

   
   Loading...