Add public API for SquashedDiff and DiffHistory.

 






 

 


  Review Request #12676
  —
  Created Oct. 7, 2022 and updated 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  RBTools
 









 
 

  release-4.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  rbtools
 









 
 

  
 






 








 





  


 
 

  
rbtools.commands.post had two classes for managing diff results:

SquashedDiff and DiffHistory.


SquashedDiff represents a single diff result for any range of commits,

without any commit history information.


DiffHistory represents commits across multiple commits, with a

cumulative diff spanning all changes.


These have been internal to the rbt post command, defined as

namedtuples (despite not used tuple functionality for these). With

some work planned for cleaning up the posting logic, it was time to

clean up these classes and move them out of this command.


Both of these now live in rbtools.clients.base.diffs. They contain

standard utility methods for building an instance from a diff result

from a SCMClient, and inherit from a base class that holds common

fields.


One API change here is that DiffHistory.cumulative_diff has been

renamed to DiffHistory.diff. This was done to simplify code that may

take either object and needs to extract the diff, and to let us leverage

as many common fields as possible.


These are still just used in rbt post, but RBTools 5 will begin to

make use of these in other modules.

 








  


 
 

  
Unit tests pass.


Successfully posted using both commit histories and squashed diffs.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Add public API for SquashedDiff and DiffHistory.
      
`rbtools.commands.post` had two classes for managing diff results:
`SquashedDiff` and `DiffHistory`.

`SquashedDiff` represents a single diff result for any range of commits,
without any commit history information.

`DiffHistory` represents commits across multiple commits, with a
cumulative diff spanning all changes.

These have been internal to the `rbt post` command, defined as
`namedtuples` (despite not used tuple functionality for these). With
some work planned for cleaning up the posting logic, it was time to
clean up these classes and move them out of this command.

Both of these now live in `rbtools.clients.base.diffs`. They contain
standard utility methods for building an instance from a diff result
from a SCMClient, and inherit from a base class that holds common
fields.

One API change here is that `DiffHistory.cumulative_diff` has been
renamed to `DiffHistory.diff`. This was done to simplify code that may
take either object and needs to extract the diff, and to let us leverage
as many common fields as possible.

These are still just used in `rbt post`, but RBTools 5 will begin to
make use of these in other modules.

     


    		
    70b4af6c329e5e8fc1464216ed685e4e38343c6f

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
'dataclasses.dataclass' imported but unused Column: 1 Error code: F401

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
undefined name 'Optional' Column: 24 Error code: F821

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
undefined name 'Optional' Column: 23 Error code: F821

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Does this Version Added: still make sense here?

      		
      
       maubinmaubin
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
    

  • Removed an unused import.
    • 

  • Added missing Optional imports (didn't affect runtime, just type checking).
    • 






 



  

   
Commits:

   
    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Add public API for SquashedDiff and DiffHistory.
      
`rbtools.commands.post` had two classes for managing diff results:
`SquashedDiff` and `DiffHistory`.

`SquashedDiff` represents a single diff result for any range of commits,
without any commit history information.

`DiffHistory` represents commits across multiple commits, with a
cumulative diff spanning all changes.

These have been internal to the `rbt post` command, defined as
`namedtuples` (despite not used tuple functionality for these). With
some work planned for cleaning up the posting logic, it was time to
clean up these classes and move them out of this command.

Both of these now live in `rbtools.clients.base.diffs`. They contain
standard utility methods for building an instance from a diff result
from a SCMClient, and inherit from a base class that holds common
fields.

One API change here is that `DiffHistory.cumulative_diff` has been
renamed to `DiffHistory.diff`. This was done to simplify code that may
take either object and needs to extract the diff, and to let us leverage
as many common fields as possible.

These are still just used in `rbt post`, but RBTools 5 will begin to
make use of these in other modules.

     


    		
    b58da8a6271b8f6b6b52448339e82edff4998e66

   




   


    

    

     

      Add public API for SquashedDiff and DiffHistory.
      
`rbtools.commands.post` had two classes for managing diff results:
`SquashedDiff` and `DiffHistory`.

`SquashedDiff` represents a single diff result for any range of commits,
without any commit history information.

`DiffHistory` represents commits across multiple commits, with a
cumulative diff spanning all changes.

These have been internal to the `rbt post` command, defined as
`namedtuples` (despite not used tuple functionality for these). With
some work planned for cleaning up the posting logic, it was time to
clean up these classes and move them out of this command.

Both of these now live in `rbtools.clients.base.diffs`. They contain
standard utility methods for building an instance from a diff result
from a SCMClient, and inherit from a base class that holds common
fields.

One API change here is that `DiffHistory.cumulative_diff` has been
renamed to `DiffHistory.diff`. This was done to simplify code that may
take either object and needs to extract the diff, and to let us leverage
as many common fields as possible.

These are still just used in `rbt post`, but RBTools 5 will begin to
make use of these in other modules.

     


    		
    70b4af6c329e5e8fc1464216ed685e4e38343c6f

   



  
 







   		
  




  

   
Diff:

   
    
 Revision 2 (+1122 -520)
Show changes
 
    docs/rbtools/api/coderef/index.rst 
    rbtools/clients/base/diffs.py 
    rbtools/clients/base/scmclient.py 
    rbtools/clients/tests/test_diff_history.py 
    rbtools/clients/tests/test_squashed_diff.py 
    rbtools/commands/post.py 
    rbtools/commands/tests/test_post.py 
 

   		
  



 









 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.
 





 















  

 
















 





 

  

   




maubin




   

   







  

 



 

  


   

   

    
 
maubin


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      rbtools/clients/base/diffs.py
      


       (Diff revision 2)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Does this Version Added: still make sense here?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 









     

    

   

  

  

   
   Loading...