Add post-commit support for BitBucket

Review Request #7513 — Created July 10, 2015 and submitted

brennie
Review Board
release-2.5.x
reviewboard

Post-commit review request creation is now enabled for the BitBucket
hosting service. The BitBucket hosting service class is now able to
fetch lists of branches, lists of change metadata, and individual diffs
from the BitBucket API.

BitBucket's API does not support fetching files by their blob IDs and
the base_commit_id parameter (which indicates the commit ID of the
revision we want the file at) must be used. This field was added to the
reviewboard.scmtools.core.Commit class so that it could be used when
fetching commits from the API and to create review requests. Without
this parameter, review request creation will fail (as all file
existence checks will fail).

Added a BitBucket hosted repository on my development server. I was
able to create post-commit review requests.

  • 0
  • 0
  • 10
  • 0
  • 10
Description From Last Updated
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
  2.  'markdown_escape' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
  2. 
      
brennie
chipx86
  1. 
      
  2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     
     
     

    We repeat the path building for repositories/%s/%s/ multiple times. Let's pull that part out into a utility function. Maybe build_repository_api_path, which could take the repository and anything that would appear after that base path.

  3. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     

    Rather than making these calls like this, can you separate them out into:

    rsp = self._api_get(url)
    
    return rsp['name']
    

    While more lengthy, it's also more readable and easier to debug this way. There's two distinct operations:

    1. We're fetching API data.
    2. We're extracting a value and returning it.

    In the event that there's an exception (let's say a KeyError) pointing to one of these lines, we'll know that it's due to either the API fetching, or the key extraction, at a glance.

    Same below.

  4. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     

    This is an even better example of where separating is going to be easier to debug if things ever go wrong, since Python considers this entire expression to be one statement and will place the exception's line at the last line of this.

  5. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     
     

    We're doing this or twice. How about:

    start = start or branch
    
    if start:
        ...
    
  6. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     

    Can you use keyword arguments for each parameter to Commit?

  7. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     

    This is worth having a documented utility function for, as it's non-obvious what's going on.

  8. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     

    Can you add a comment saying that the commit will be added to the change in the call to get_commits? I had to dig through the code to figure that out (originally I thought we were missing a call).

  9. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
     
     

    Same here about keyword arguments.

  10. 
      
chipx86
  1. Oh, can you also flesh out the description a bit? This should go into more detail on what this allows, briefly what's involved in getting it to work, and the work needed to plumb the base_commit_id stuff through.

  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
  2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 3)
     
     
    Col: 5
     E303 too many blank lines (2)
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/models/base_review_request_details.py
        reviewboard/scmtools/core.py
        reviewboard/hostingsvcs/bitbucket.py
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (478c095)
Loading...