• 
      

    Add support for reading and writing history metadata

    Review Request #8278 — Created July 11, 2016 and submitted

    Information

    RBTools
    dvcs
    4f3c6e5...

    Reviewers

    For server-side detection of rebases and commit amends, we need to keep
    track of some metadata indicating the original SHA1 of commits. With
    git, we can store this metadata in git notes.

    git notes allows for storing arbitrary metadata associated with each
    commit. We store the SHA1 of the commit in a special notes ref,
    rbt-history-metadata. This way, we do not interfere with notes that
    users may be using. We also add this ref to the list of refs to be
    rewritten during rebases and amends.

    When an amend occurs, the SHA1 of the commit will change, but the
    stored SHA1 will not, so we will be able to detect that commit was
    amended. Likewise, when a squash occurs during a rebase, the SHA1 will
    change but the stored metadata will be the SHA1 hashes of all the
    squashed commits, so we can also know when a squash occurs. We can then
    upload this metadata when doing rbt post so that Review Board can
    detect history changes server side.

    Using https://reviews.reviewboard.org/r/8276/, I was able to do the
    following:

    • Upload a review request with multiple commits.
    • Squash the commits together locally.
    • Update the review request.

    I observed that the original commits were set in the Review Board
    database.

    Description From Last Updated

    Can we make this if commits is None instead?

    daviddavid

    Can we rearrange and simplify this a tiny bit? I don't think we need separate error messages for the two …

    daviddavid

    Instead of the early return, I think it would be better to put the logging statement into the conditional and …

    daviddavid
    brennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/commands/post.py
          rbtools/clients/__init__.py
          rbtools/api/resource.py
          rbtools/clients/git.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/commands/post.py
          rbtools/clients/__init__.py
          rbtools/api/resource.py
          rbtools/clients/git.py
      
      
    2. 
        
    brennie
    david
    1. 
        
    2. rbtools/clients/git.py (Diff revision 1)
       
       
      Show all issues

      Can we make this if commits is None instead?

    3. rbtools/clients/git.py (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Can we rearrange and simplify this a tiny bit? I don't think we need separate error messages for the two cases because the remedy is the same:

      if rewrite_mode.strip() != 'concatenate':
          logging.warn('notes.rewriteMode is not set to concatenate. '
                       'Review Board will not be able to correctly track '
                       'rebased and amended commits without this. To fix '
                       'this, run:\n'
                       '\tgit config notes.rewriteMode concatenate')
      
    4. 
        
    brennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/commands/post.py
          rbtools/clients/__init__.py
          rbtools/api/resource.py
          rbtools/clients/git.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/commands/post.py
          rbtools/clients/__init__.py
          rbtools/api/resource.py
          rbtools/clients/git.py
      
      
    2. 
        
    david
    1. 
        
    2. rbtools/clients/git.py (Diff revision 2)
       
       
       
      Show all issues

      Instead of the early return, I think it would be better to put the logging statement into the conditional and just let control flow go all the way to the end of the function. This is simplest if you don't use none_on_ignored_error in the execute call, in which case you should always have a string, but you could also test for rewrite_mode is None or rewrite_mode.strip() != 'concatenate'

    3. 
        
    brennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          rbtools/commands/post.py
          rbtools/clients/__init__.py
          rbtools/api/resource.py
          rbtools/clients/git.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          rbtools/commands/post.py
          rbtools/clients/__init__.py
          rbtools/api/resource.py
          rbtools/clients/git.py
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    brennie
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to dvcs (53e61d5)