Add support for -X/--exclude/EXCLUDE_FILES.

Review Request #6317 — Created Sept. 12, 2014 and submitted

brennie
RBTools
master
6331
422122c...
rbtools, students

rbtools now recognizes the -X and --exclude command line options for
excluding files from diffs and review requests. The EXCLUDE_FILES
option in .reviewboardrc adds the same functionality, but can be
committed to the repository.

Currently no backends support this option.

Unit tests pass.
Running rbt diff with -X or --exclude flags, or with
EXCLUDE_FILES set in .reviewboardrc causes the expected error
message

Description From Last Updated

The SCMClients shouldn't be handling these as command line arguments. Instead, diff should be updated to take an excluded_files= argument ...

chipx86chipx86

"or"

chipx86chipx86

Can we rename the existing parameter files to be include_files, and put the new parameter right after that one? Same ...

daviddavid

We prefer the % formatting scheme over this one (change "{0}" to "%s" and replace ".format(tool.name)" with "% tool.name").

daviddavid

Can you wrap the whole conditional in parens and remove the continuation character?

daviddavid

Same comment about .format vs %

daviddavid

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/commands/__init__.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.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/commands/__init__.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.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/commands/__init__.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.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
        AUTHORS
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.py
        rbtools/commands/__init__.py
        rbtools/clients/clearcase.py
        rbtools/clients/plastic.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
        AUTHORS
    
    
  2. 
      
chipx86
  1. 
      
  2. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     
     

    The SCMClients shouldn't be handling these as command line arguments. Instead, diff should be updated to take an excluded_files= argument and use that insead. We're trying to eventually move away from having self.options.

    I'm also hoping we can move away from these errors within diff. Instead, how about adding a supports_diff_exclude_files flag on the class, and having the caller check this and provide the error? (We should probably do this for include as well, but that'd be a separate change.)

  3. rbtools/clients/cvs.py (Diff revision 2)
     
     
  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.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
        AUTHORS
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.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
        AUTHORS
    
    
  2. 
      
david
  1. 
      
  2. rbtools/clients/__init__.py (Diff revision 3)
     
     

    Can we rename the existing parameter files to be include_files, and put the new parameter right after that one? Same in the rest of the clients.

  3. rbtools/commands/diff.py (Diff revision 3)
     
     
     
     
     

    We prefer the % formatting scheme over this one (change "{0}" to "%s" and replace ".format(tool.name)" with "% tool.name").

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

    Can you wrap the whole conditional in parens and remove the continuation character?

  5. rbtools/commands/post.py (Diff revision 3)
     
     

    Same comment about .format vs %

  6. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.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
        AUTHORS
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.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
        AUTHORS
    
    
  2. rbtools/clients/cvs.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/clients/cvs.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
        AUTHORS
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/clients/cvs.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
        AUTHORS
    
    
  2. 
      
david
  1. This looks good to me, but I'd like to hold off on pushing it until there's an implementation for at least one SCMClient. You can build on this by creating a new branch on top of this and then posting changes with rbt post <thisbranch>..<newbranch>

  2. 
      
chipx86
  1. Ship It!

  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

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