Add support for -X to git SCMClient

Review Request #6339 — Created Sept. 19, 2014 and submitted

brennie
RBTools
master
6345
191bc37...
rbtools, students

Git (and the git-svn and git-p4 backends) now support the -X/--exclude
commandline option and the EXCLUDE_PATTERNS .reviewboardrc option.

Add unit tests for excluding files from Git diffs.

Rename exclude_files/EXCLUDE_FILES to exclude_patterns/EXCLUDE_PATTERNS
to reflect the actual behaviour of -X.

Unit tests pass.

Generated diffs with excluded files via rbt diff with git, git-svn,
and git-p4.

  • 0
  • 0
  • 16
  • 0
  • 16
Description From Last Updated
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tests.py
        rbtools/clients/git.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tests.py
        rbtools/clients/git.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. rbtools/clients/git.py (Diff revision 1)
     
     
    Col: 59
     E231 missing whitespace after ','
    
  3. rbtools/clients/git.py (Diff revision 1)
     
     
    Col: 39
     E127 continuation line over-indented for visual indent
    
  4. rbtools/clients/git.py (Diff revision 1)
     
     
    Col: 39
     E127 continuation line over-indented for visual indent
    
  5. rbtools/clients/git.py (Diff revision 1)
     
     
    Col: 39
     E127 continuation line over-indented for visual indent
    
  6. rbtools/clients/git.py (Diff revision 1)
     
     
    Col: 39
     E127 continuation line over-indented for visual indent
    
  7. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/tests.py
        rbtools/clients/git.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/tests.py
        rbtools/clients/git.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
chipx86
  1. 
      
  2. rbtools/clients/git.py (Diff revision 2)
     
     

    Can you rename to filename? file is reserved in Python.

  3. rbtools/clients/git.py (Diff revision 2)
     
     
     
     

    I assume this is an error condition? If so, we should probably log something.

  4. rbtools/clients/git.py (Diff revision 2)
     
     
     
     

    No blank line here.

  5. rbtools/clients/git.py (Diff revision 2)
     
     
     

    Any chance of supporting both include and exclude at the same time? (Did the Mercurial change support this?)

    1. By this do you mean removing files in exclude_files from include_files?

      The mercurial change just adds -X f1 -X f2 to the commandline because it natively supports it.

    2. If you have excluded files set up, then no included files passed on the command line will be used, since we never reach this block.

      What does Mercurial do if you specify both specific files and use -X?

    3. As it turns out, hg diff -r r1 -r r2 -X f -- f returns nothing. I /think/ this is a good behaviour and will replicate this in the git tool.

  6. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. rbtools/clients/git.py (Diff revision 3)
     
     
     local variable 'include_Files' is assigned to but never used
    
  3. 
      
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
  1. 
      
  2. rbtools/clients/tests.py (Diff revision 4)
     
     

    Can you tell me which assert statements actually test your changes? I'm having trouble understainding the test case

    1. The test compares the result of running the git diff after including two files and excluding one of them to what the result would be of just including one file.

  3. 
      
  1. 
      
  2. 
      
  1. Ship It!

  2. 
      
david
  1. 
      
  2. docs/rbtools/rbt/commands/post.rst (Diff revision 4)
     
     

    This should be updated to say "pattern" instead of "file".

  3. rbtools/clients/git.py (Diff revision 4)
     
     
     
     
     
     
     

    This should probably have a final 'else' that just asserts.

  4. rbtools/clients/git.py (Diff revision 4)
     
     
     
     
     

    Can we log this message at a higher level than 'debug'?

  5. rbtools/clients/git.py (Diff revision 4)
     
     
     
     

    Please replace the `' with "" and put this whole string in single-quotes instead of double.

  6. rbtools/commands/diff.py (Diff revision 4)
     
     

    This line should be indented one more space (to match up with the '(').

  7. rbtools/commands/post.py (Diff revision 4)
     
     

    This should be indented one more space.

  8. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
  1. 
      
  2. rbtools/clients/git.py (Diff revision 4)
     
     
     
     
     
     
     
     

    You can use fnmatch.filter function to get a subset of the filenames that match each pattern. It will simplify the code a bit but not necessary.

    1. Implemented in latest diff.

  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/clients/tests.py
        rbtools/commands/post.py
        rbtools/commands/__init__.py
        rbtools/commands/diff.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.py
        rbtools/clients/__init__.py
        rbtools/clients/mercurial.py
        rbtools/clients/git.py
        rbtools/clients/perforce.py
        rbtools/clients/bazaar.py
        rbtools/clients/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
david
  1. Ship It!

  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.6.x (f7a6d7d)
Loading...