Add mercurial generated git diff parser.

Review Request #7264 — Created April 29, 2015 and submitted

Information

Review Board
release-2.0.x
d39b867...

Reviewers

This adds a new HgGitDiffParser which will parse the headers generated
by "hg export --git" and then rely on the GitDiffParser to parse the
diff contents.

The logic for deciding which hg parser to use has also been changed
slightly so that it takes into account the possible presence of the
commented headers hg provides.

Tests for mercurial git diff parsing have been updated to provide the
proper extended headers.

New tests, unit tests pass, uploaded diff from "hg export --git" parses and displays.

Description From Last Updated

're' imported but unused

reviewbotreviewbot

Small thing, but can you change the 0-related comparisons to check against -1 instead? I think that will help differentiate …

chipx86chipx86

These should be moved into __init__.

brenniebrennie

Needs a docstring.

chipx86chipx86

Needs a docstring.

chipx86chipx86

Should we have an else statement that does something? At the very least, log a warning, or may raise an …

chipx86chipx86

Needs a docstring.

chipx86chipx86

Col: 9 E303 too many blank lines (2)

reviewbotreviewbot

Blank line after this.

chipx86chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/scmtools/tests.py
        reviewboard/scmtools/hg.py
    
    
  2. reviewboard/scmtools/hg.py (Diff revision 1)
     
     
     're' imported but unused
    
  3. 
      
brennie
  1. 
      
  2. reviewboard/scmtools/hg.py (Diff revision 1)
     
     
     
     

    These should be moved into __init__.

  3. 
      
chipx86
  1. Thinking it's worth adding some unit tests to cover the relative positions of diff -r and diff --git, and to also ensure that the new_commit_id and base_commit_id results are what we expect in whatever cases could come up.

  2. reviewboard/scmtools/hg.py (Diff revision 1)
     
     
     

    Small thing, but can you change the 0-related comparisons to check against -1 instead? I think that will help differentiate a little between "is it found/not found" vs. "position relative to the other string" when reading the code.

  3. reviewboard/scmtools/hg.py (Diff revision 1)
     
     

    Needs a docstring.

  4. reviewboard/scmtools/hg.py (Diff revision 1)
     
     

    Needs a docstring.

  5. reviewboard/scmtools/hg.py (Diff revision 1)
     
     
     
     
     

    Should we have an else statement that does something? At the very least, log a warning, or may raise an exception if the format is invalid?

    1. If neither is found the format is still valid. There are other headers HG is providing as well but we ignore them - ex:

      # HG changeset patch
      # User Steven MacLeod <steven@smacleod.ca>
      # Date 1429633205 14400
      #      Tue Apr 21 12:20:05 2015 -0400
      # Node ID 6187592a72d7bd1e147a180102ad91522eb6f024
      # Parent  9d3f4147f2943856c4f9be791b41525215cfc706
      

      Going to drop this.

  6. reviewboard/scmtools/hg.py (Diff revision 1)
     
     

    Needs a docstring.

  7. 
      
SM
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/scmtools/tests.py
        reviewboard/scmtools/hg.py
    
    
  2. reviewboard/scmtools/tests.py (Diff revision 2)
     
     
    Col: 9
     E303 too many blank lines (2)
    
  3. 
      
SM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/scmtools/tests.py
        reviewboard/scmtools/hg.py
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/scmtools/hg.py (Diff revision 3)
     
     

    Blank line after this.

  3. 
      
SM
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/scmtools/tests.py
        reviewboard/scmtools/hg.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/scmtools/tests.py
        reviewboard/scmtools/hg.py
    
    
  2. 
      
SM
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (f71fb32)
Loading...