ClearCase  UnboundLocalError instead communicate with informations

 






 

 


  Review Request #1508
  —
  Created April 6, 2010 and discarded

 


















 

  

   Information

  





 
 

  jan.koprowski
 









 
 

  RBTools
 









 
 

  master
 









 
 

  1599
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
When there is some problems with view open function just raise an exception instead return some human readable information

 








  


 
 

  

 







 









 





  








 









 






  

 








 














 





 

  

   





 DJ





   

   





  

 



 

  


   

   

    
 
djs


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Use double quotes instead of escapes if needed.

Grammar:

File %r does not exist or unknown viewtype (%s)
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      "unknown" viewtype is not the same as "viewtype is not adequat". There is extend_path in ClearCase like filename.ext@@/main/branch/VERSION and some of them work only in dynamic view. So You can search file /my/development/path/filename.ext@@/main/branch/VERSION in "snapshot" view and You get error but not because viewtype is unknown but is not adequat (should be dynamic).

This is mistake which I made many times when I starting with post-review. I just forgot to change viewtype in post-review.
From the other hand maybe this is good moment to add parameter "--view-type"?
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




jan.koprowski




   

   

  

 



 















 



 

  

   




jan.koprowski




   

   

  

 



 















 



 

  

   




jan.koprowski




   

   

  

 



 















 





 

  

   





 DJ





   

   





  

 



 

  


   

   

    
 
djs


    
   


  

  

  



    




  1. 
 
    
  
    
   
    The word should be 'adequate' not 'adequat'. I suggested 'unknown' because I'm not quite sure what you were trying to suggest. I have some knowledge of CC and have tested RB with CC, but I don't normally use, so I'm not quite sure what state everything is in. Last I checked, I could only get post-review to work with dynamic views. Is your error message meant to indicate this? I imagine we could detect the type of view with a cleartool command, instead of specifying it as an option.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      1) Specifying viewtype using cleartool is good idea, I will try do this.
2) Post-review works only with dynamic view now. Question is there can work with snapshots? Ever? For my knowledge (I'm not CC expert just maybe a little bit geeky user) paths like @@/main etc works only in dynamic view. Without that support in snapshots we can't get appropriate informations because @@ path wasn't work (I may be in mistake). Before I add --view-type "snapshot" was hardocoded. When "snapshot" was hardcoded and I use post-review in dynamic view I just get error because none of conditions was true and variable "fn" wasn't initialized. So I decide add else clause which return error which mean: You use post-review with --view-type = dynamic in snapshot view or you use post-review with --view-type = snapshot in dynamic view. This is the goal of this else clause.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      That is my understanding as well. The extended path notation is provided by the special filesystem which only works if you're using that filesystem. For snapshot views, we'd need to get the file data from elsewhere. I don't have any idea how (other than editing the config spec), but I imagine there is probably a way.

What confused me is what the original author of the clearcase support was thinking, since the code implied to me that it does support snapshot views, somehow, especially given 'snapshot' is the default type of view that was hardcoded...
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      So. Summaries.
1) Change hard coded information "snapshot" to "dynamic" is a good idea, because now hardcoded just make post-review useless as default for ClearCase.
2) For now we know dynamic ClearCase way to get needed informations about files but there is a loooong way to get post-review works with snapshot views as good as dynamic view work, and we didn't know how to do that now.
3) This will be good to check viewtype using cleartool or even back die("ReviewBoard post-review script does not work with snapshot views in this version. Please use dynamic view.") when was run in snapshot.
4) For now parameter --view-type is stupid thing (because one option doesn't work) so meybe hardcoded value even will be better but this parameter is very prospective for now (until auto recognized isn't work). You decide what should stay.

I think I don't have enough time for this issues in my work (else much more important waiting). I try back to this in the future. For now I will stay this like this. In this or next week I try update documentation about CC in git.
      
 
      

      4. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




jan.koprowski




   

   

  

 



 















 





 

  

   





 DJ





   

   





  

 



 

  


   

   

    
 
djs


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 6)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Almost... still a typo here. Should be 'adequate'
    
  
    
 
    

 
    
  



 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




jan.koprowski




   

   

  

 



 















 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 7)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Can you rename this to --clearcase-view-type?
    
  
    
 
    

 
    
  



 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 7)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    The variable should be named "clearcase_view_type".

Would it make sense to default this parameter to "snapshot" for existing users?
    
  
    
 
    

 
    
  



 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 7)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    "The view type used when generating the diff (ClearCase Only)"
    
  
    
 
    

 
    
  



 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 7)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Multi-line conditionals should use (...) instead of \
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      One condition behind that use \ not (...)  so I use \.  I change to one line.
      
 
      

      2. 


 
    


    

 
    

    6. 






  6. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 7)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    This makes it sound like the parameter doesn't exist. The error message should better describe that the view type isn't set and needs to be. Something like:

"The --clearcase-view-type parameter must be specified for ClearCase repositories."
    
  
    
 
    

 
    
  



 
    

    7. 




  7. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    8. 




  

 
















 



 

  

   




jan.koprowski




   

   

  

 



 















 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 8)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    This is going to break some existing setups, I'd imagine. I'd be much happier if this could work like repository information for other SCMs. Make it a customizable setting in .reviewboardrc for the repository, so that the user will only have to pass it if it's not found there.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I have no known about any Clear Case ability to keep additional informations or any meta-data. I will ask in my company but for all I know Clear Case can't do that.
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




jan.koprowski




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
Only dynamic view is able to show any version existing in repository. Snapshot view is unable to be used as information about other versions then checked out (this is like a photo of the repository take in selected moment. You can't see what was happen before or after). Only way to implement snapshot views in ClearCase is to add something like --compare-directories="/path/to/directory1:/path/to/directory2". I show similar implementation in one compering tool which get two distinct snapshot views to compare them.

   

  








  

   

    Diff:
   

   

    
 Revision 9 (+4 -1)
Show changes
 
    rbtools/postreview.py