Add -X support for the svn SCMClient.

Review Request #6345 — Created Sept. 20, 2014 and submitted

Information

RBTools
master
5a8fae4...

Reviewers

The svn backend now supports excluding files through a
filterdiff-esque method: SVNClient._filter_diff. This
method looks through the output of svn diff for lines
that begin with Index: that mark the beginning of a
new file in the diff. If the file matches any given
patterns, it will not yield the lines corresponding to
that file's diff.

Ran rbt diff -X ... in an SVN repository and successfully
excluded files and empty files.

Description From Last Updated

If for some reason there's text at the beginning of the diff before the first index line (which is valid), …

daviddavid

Should have a trailing period. Can you add more details to this comment to describe what's happening in the change? …

chipx86chipx86

Blank line between statements and blocks.

chipx86chipx86

Here too. Also, minor thing, but we usually just use m for this variable. Not a big deal, but doesn't …

chipx86chipx86

And here.

chipx86chipx86

What about on Windows?

chipx86chipx86

Summary should be on the same line as the """.

chipx86chipx86
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        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/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
brennie
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        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/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
david
  1. 
      
  2. rbtools/clients/svn.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    If for some reason there's text at the beginning of the diff before the first index line (which is valid), it will cause a NameError here, since include_file hasn't yet been set. Can you set include_file = True at the top of this function?

  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        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/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
chipx86
  1. 
      
  2. rbtools/clients/svn.py (Diff revision 3)
     
     
    Show all issues

    Should have a trailing period.

    Can you add more details to this comment to describe what's happening in the change?

    I'm also interested in why we're filtering and not using -x. Did it turn out that we couldn't pass that to SVN?

    1. SVN calls out to external diff for each file, so if we run svn diff --diff-cmd=diff '-u -x pattern, then it runs a command like:
      diff -L "f (rev x)" -L "f (rev y)" old new -x pattern f. However, if f matches the given pattern, it will still spit out the diff becuase it was called on that specific file (not with, e.g., diff -x pattern -r old/ new/).

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

    Blank line between statements and blocks.

  4. rbtools/clients/svn.py (Diff revision 3)
     
     
     
    Show all issues

    Here too.

    Also, minor thing, but we usually just use m for this variable. Not a big deal, but doesn't hurt to be consistent.

  5. rbtools/clients/svn.py (Diff revision 3)
     
     
     
    Show all issues

    And here.

  6. rbtools/clients/svn.py (Diff revision 3)
     
     
    Show all issues

    What about on Windows?

    1. It turns out its not from svn diff. Its from SCMClient.convert_to_absolute_files.
      It will always prepend the slash.

  7. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        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/svn.py
    
    Ignored Files:
        docs/rbtools/rbt/commands/post.rst
        docs/rbtools/rbt/commands/diff.rst
    
    
  2. 
      
chipx86
  1. One last thing!

  2. rbtools/clients/svn.py (Diff revision 4)
     
     
     
    Show all issues

    Summary should be on the same line as the """.

  3. 
      
chipx86
  1. Ship It!

  2. 
      
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to release-0.6.x (f4b3936)