Add full-repository support for Review Bot tools with git.

Review Request #8817 — Created March 15, 2017 and submitted

Information

ReviewBot
master
678a0a2...

Reviewers

This change adds a way for Review Bot tools to operate with an entire checkout
of a (git) repository. This requires a fair amount of configuration on the
worker side, which I'm planning to document more thoroughly in a forthcoming
user manual.

When one or more repositories is configured, the worker will listen on a new
set of queues (the product of the list of repositories and the available tools
which can run on full repos). This way, a worker will only pick up tasks for
repositories which it has available.

When a change using a repository is triggered for review, the repository is
either cloned (if it doesn't exist), or fetched. This repo is then
shallow-cloned to the desired parent revision and the patched files are
downloaded. The tool can then do whatever it needs with the repo and post its
review.

  • Verified that clone/fetch and the shallow cloned worked as expected.
  • Used this in conjunction with a Clang Static Analyzer tool.
Description From Last Updated

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (85 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (83 > 79 characters)

reviewbotreviewbot

'os' imported but unused

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

undefined name 'logging'

reviewbotreviewbot

undefined name 'logging'

reviewbotreviewbot

You don't need .keys() to iterate over the dict's keys. Iterating over it will yield the keys.

brenniebrennie

This is going to get big over time as we add support for more types. Let's split this up now, …

chipx86chipx86

Clang?

chipx86chipx86

Is there any mechanism to send parameters using something akin to keyword arguments? Maybe send a dictionary across? I worry …

chipx86chipx86

Unnecessary blank line.

brenniebrennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tasks.py
        bot/reviewbot/tools/__init__.py
        bot/reviewbot/repositories.py
        extension/reviewbotext/integration.py
        bot/reviewbot/utils/filesystem.py
        bot/reviewbot/tools/buildbot.py
        bot/reviewbot/celery.py
        extension/reviewbotext/resources.py
        extension/reviewbotext/models.py
        bot/reviewbot/config.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tasks.py
        bot/reviewbot/tools/__init__.py
        bot/reviewbot/repositories.py
        extension/reviewbotext/integration.py
        bot/reviewbot/utils/filesystem.py
        bot/reviewbot/tools/buildbot.py
        bot/reviewbot/celery.py
        extension/reviewbotext/resources.py
        extension/reviewbotext/models.py
        bot/reviewbot/config.py
    
    
  2. bot/reviewbot/celery.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  3. bot/reviewbot/repositories.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  4. bot/reviewbot/repositories.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (85 > 79 characters)
    
  5. bot/reviewbot/tasks.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  6. bot/reviewbot/tasks.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (83 > 79 characters)
    
  7. bot/reviewbot/tools/__init__.py (Diff revision 1)
     
     
    Show all issues
     'os' imported but unused
    
  8. bot/reviewbot/tools/buildbot.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  9. bot/reviewbot/utils/filesystem.py (Diff revision 1)
     
     
    Show all issues
     undefined name 'logging'
    
  10. bot/reviewbot/utils/filesystem.py (Diff revision 1)
     
     
    Show all issues
     undefined name 'logging'
    
  11. 
      
david
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tasks.py
        bot/reviewbot/tools/__init__.py
        bot/reviewbot/repositories.py
        extension/reviewbotext/integration.py
        bot/reviewbot/utils/filesystem.py
        bot/reviewbot/tools/buildbot.py
        bot/reviewbot/celery.py
        extension/reviewbotext/resources.py
        extension/reviewbotext/models.py
        bot/reviewbot/config.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tasks.py
        bot/reviewbot/tools/__init__.py
        bot/reviewbot/repositories.py
        extension/reviewbotext/integration.py
        bot/reviewbot/utils/filesystem.py
        bot/reviewbot/tools/buildbot.py
        bot/reviewbot/celery.py
        extension/reviewbotext/resources.py
        extension/reviewbotext/models.py
        bot/reviewbot/config.py
    
    
  2. 
      
brennie
  1. 
      
  2. bot/reviewbot/celery.py (Diff revision 2)
     
     
    Show all issues

    You don't need .keys() to iterate over the dict's keys. Iterating over it will yield the keys.

  3. 
      
chipx86
  1. 
      
  2. bot/reviewbot/repositories.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    This is going to get big over time as we add support for more types. Let's split this up now, have a directory for repositories and a module for the base, another for git.

    1. I think I'd prefer to split it up when it actually gets big. This whole file is just 102 lines long.

  3. bot/reviewbot/tasks.py (Diff revision 2)
     
     
    Show all issues

    Clang?

  4. extension/reviewbotext/integration.py (Diff revision 2)
     
     
     
     
     
     
     
    Show all issues

    Is there any mechanism to send parameters using something akin to keyword arguments? Maybe send a dictionary across? I worry about expanding this down the road and introducing compatibility nightmares.

    1. Hmm. I'm not sure. I'd really rather not deal with that now, though. I'll make a note to investigate.

  5. 
      
david
brennie
  1. 
      
  2. bot/reviewbot/celery.py (Diff revision 3)
     
     
    Show all issues

    Unnecessary blank line.

  3. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to master (32d991b)