Add javascript models for Repository, Branch, and Commit.

 






 

 


  Review Request #4266
  —
  Created June 26, 2013 and submitted 

 


















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Add javascript models for Repository, Branch, and Commit.

This change adds models and collections for repositories, branches, and commits,
along with basic tests for the fetching and parsing logic. This implements the
client side of the new commits and branches resources.


 








  


 
 

  
Ran unit tests. Used these models in my larger post-commit changes.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
If these are instance variables, you don't want them here. They should be set in initialize(). Every object should specifically …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
This is just testing that we set it above. Doesn't seem to be very useful as-is.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Probably, we should just use options.blah instead of setting these values. Then you'd define: collection = new RB.RepositoryCommits([], { urlBase: …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Did you mean to leave this?

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Can you wrap this?

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Every Model has an id field, so we don't need this. By setting id below, the model's ID will be …

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
Missing ;

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
I feel like this should be a Resource model. I don't know that it's really worth having inconsistent modelling for …

      		
      
       chipx86chipx86
      
      
       
      
     


    
   

  

 







 






  

 








 














 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    If these are instance variables, you don't want them here. They should be set in initialize().

Every object should specifically set every instance variable it will use in its lifetime in a default value in initialize(). Any object in JavaScript that dynamically defines new variables on itself post-initialize takes a performance and memory hit, since the VM can't share as much state. (There was a good writeup on this somewhere, but I can't find it off-hand.)
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This is just testing that we set it above. Doesn't seem to be very useful as-is.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Probably, we should just use options.blah instead of setting these values. Then you'd define:

collection = new RB.RepositoryCommits([], {
    urlBase: url,
    start: start
});

The collection would then use this.options.urlBase/start instead of this.urlBase/start, which is more Backbone-ish.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Did you mean to leave this?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 






  6. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Can you wrap this?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    7. 






  7. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Every Model has an id field, so we don't need this. By setting id below, the model's ID will be the provided ID. I think that's okay, but worth noting.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    8. 






  8. 
 
    
  
  
    
  
 

   
    
    
    





    
    Any risk of this failing?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      In the case where there's nowhere to split, it will just return a single-item array with the string itself, which matches the way we handle single-line commit messages in the perforce change description parsing.
      
 
      

      2. 


 
    


    

 
    

    9. 






  9. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Missing ;
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    10. 






  10. 
 
    
  
  
    
  
 

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    I feel like this should be a Resource model. I don't know that it's really worth having inconsistent modelling for these things.

I also want to start putting everything api-related in resources/{collections,models}/, so I think this and the other resource models/collections should go in there.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I'm really kind of wary about pulling in all the complexity of BaseResource, since at the moment this will never load from or save to the server, and is really just a place to aggregate the repository ID, branches, and commits. Using BaseResource means that in order to get a url I have to set a parent object and deal with ready().
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      I'm going to need it for Repository. The new smarter repository creator will need to load in resources and create them, so the complexity's going to be used there at least.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      If you want me to do the conversion at a later point, I can.
      
 
      

      4. 


 
    


    

 
    

    11. 




  11. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    12. 




  

 
















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 




 
Change Summary:

 
Pushed to master (20024e9).





 


 










  

 









     

    

   

  

  

   
   Loading...