• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    Add ability for Review Bot tools to attach logs
    
 
    

    



 
    
 
    

  Review Request #10318
  —
  Created Nov. 8, 2018 and discarded 

 
    


    




    




    




 
    
  
    
   Information

  
    



    
 
 
    
  ilaw
 
    

    





    
 
 
    
  ReviewBot
 
    

    





    
 
 
    
  master
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  
 
    

    



 
    



 
    
  
    
   Reviewers

  
    



    
 
 
    
  reviewbot, students
 
    

    





    
 
 
    
  
 
    

    



 
    



    


    
 
    




  

    
 
 
    
  
    Allows Review Bot tools to attach their console output to Status Updates. 
    

    Adds ability to attach output files to checkstyle, Cppcheck, CppLint, doc8, 
    
flake8, JSHint, PMD, pycodestyle, pydocstyle, and pyflakes. 
    

    Adds an option to the above-mentioned Review Bot tools to attach the output 
    
files. The default is set to not attach files.
    
 
    

    





  

    
 
 
    
  
    Tested ability to attach log files by configuring integrations with the 
    
Review Bot tools, running each tool, and examining the attachments.
    
 
    

    




 
    

    





    
 
    









 





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



     
    
      
       
       
    Parentheses are not needed here.
    
          		
      
       brenniebrennie
      
      
       
      
     
    



     
    
      
       
       
    You are running the tool twice here.
    
          		
      
       brenniebrennie
      
      
       
      
     
    



     
    
      
       
       
    E501 line too long (83 > 79 characters)
    
          		
      
       reviewbotreviewbot
      
      
       
      
     
    



     
    
      
       
       
    It might be helpful to add a comment explaining what the regex does so it's easier to read this code …
    
          		
      
       shovenshoven
      
      
       
      
     
    



     
    
      
       
       
    W292 no newline at end of file
    
          		
      
       reviewbotreviewbot
      
      
       
      
     
    



     
    
      
       
       
    For this (and the other structures like it), since these ifs have no elses, would it be worth making it …
    
          		
      
       gojeffchogojeffcho
      
      
       
      
     
    

        
   
    
  
    
 
    

    




 
    

    



  
    
 
    

    


    












    
 




 

    


 













    
 





 
    
  
    
   
    



brennie




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
brennie


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 

















    2. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       bot/reviewbot/tools/cppcheck.py
       


        (Diff revision 1)


       
      
     
      
          
    
     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   Show all issues
  
 
      

      



    
      Parentheses are not needed here.
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 






    3. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       bot/reviewbot/tools/cppcheck.py
       


        (Diff revision 1)


       
      
     
      
          
    
     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   Show all issues
  
 
      

      



    
      You are running the tool twice here.
      
  
      
 
      

 
      
  


      
 
      
 
        

  


      1. 
 
 
        
 
        
  
 
        
 
        
  
        I've changed it so it joins the lines together instead, is there a better way to do this?
        
 
        

        2. 



      2. 
 
 
        
 
        
  
 
        
 
        
  
        LGTM
        
 
        

        3. 


 
      


      

 
      

      4. 




    4. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      5. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



ilaw




   
    
   
    
  
    
 
    


 

    












    
 



 
    
  
    
   
    



ilaw




   
    
   
    
  
    
 
    


 
    
  
    

   
    
   
    
    
     Review request changed
    
    
   
    

  
    
  
    
  
    




    









  
    
   
    
    Summary:
   
    
   
    
    
    [WIP] Allow Review Bot tools to attach logs
    Add ability for Review Bot tools to attach logs
    
   
    
  
    




  
    
   
    
    Description:
   
    
   
    
    
     ~   
    Testing attach logs with cppcheck
       ~ 
    Allows Review Bot tools to attach their console output to Status Updates. 
       + 

       + 
    Adds ability to attach output files to checkstyle, Cppcheck, CppLint, doc8, 
       + flake8, JSHint, PMD, pycodestyle, pydocstyle, and pyflakes. 
       + 

       + 
    Adds an option to the above-mentioned Review Bot tools to attach the output 
       + files. The default is set to not attach files.
    
   
    
  
    




  
    
   
    
    Testing Done:
   
    
   
    
    
       + 
    Tested ability to attach log files by configuring integrations with the 
       + Review Bot tools, running each tool, and examining the attachments.
    
   
    
  
    




  
    
   
    
    Commit:
   
    
   
    
    
    ff08474358b30156771ded24f084b4adb2296f03
    0c4b6e5da1f20d328464fbdc393c3aa0994dcb60
    
   
    
  
    




  
    
   
    
    Diff:
   
    
   
    
    
     Revision 3 (+251 -3)
    Show changes
     
        bot/reviewbot/tools/checkstyle.py 
        bot/reviewbot/tools/cppcheck.py 
        bot/reviewbot/tools/cpplint.py 
        bot/reviewbot/tools/doc8.py 
        bot/reviewbot/tools/flake8.py 
        bot/reviewbot/tools/jshint.py 
        bot/reviewbot/tools/pmd.py 
        bot/reviewbot/tools/pycodestyle.py 
      2 more
     
    
   
    
  
    





    



    
 
    
  
    Checks run (1 failed, 1 succeeded)
    


  
    
 flake8
 failed.
 


    


  
    
 JSHint
 passed.
 


    


 
    





    
 
    flake8
    
 
    

 

    








    




  
    
 
    

    












    
 





 
    
  
    
   
    



shoven




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
shoven


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 

















    2. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       bot/reviewbot/tools/checkstyle.py
       


        (Diff revision 3)


       
      
     
      
          
    
     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   Show all issues
  
 
      

      



    
      It might be helpful to add a comment explaining what the regex does so it's easier to read this code on first glance if one is not as familiar with regex.
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 




    3. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      4. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



ilaw




   
    
   
    
  
    
 
    


 

    












    
 





 
    
  
    
   
    



gojeffcho




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
gojeffcho


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 

















    2. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       bot/reviewbot/tools/cppcheck.py
       


        (Diff revision 4)


       
      
     
      
          
    
     
       
      

     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    




      
 
      
  
  
  

  
  
   Show all issues
  
 
      

      



    
      For this (and the other structures like it), since these ifs have no elses, would it be worth making it if settings['attach_log'] and output:?
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3. 




    3. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      4. 


    

  
    
 
    

    












    
 



 
    
  
    
   
    



david




   
    
   
    
  
    
 
    


 
    
  
    

   
    
   
    
    
     Review request changed
    
    
   
    

  
    
  
    
  
    




    

  
    
   
    
    Status:
   
    
   
    

    Discarded