Add --no-renames flag for git diffs from rbt diff.

Review Request #9221 - Created Sept. 24, 2017 and updated

Brian LeBlanc
RBTools
release-0.7.x
4551
276ea82...
rbtools, students
chipx86, david

When creating a git diff with rbt diff, the -M option is automatically
used if available and the user has no way of making rbt pass in
--no-renames to git diff.

This change adds a --no-renames flag to the rbt diff command which then
gets passed into git diff.

Tests were run.

  • 0
  • 0
  • 11
  • 0
  • 11
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

Brian LeBlanc
Barret Rennie
  1. 
      
  2. rbtools/clients/git.py (Diff revision 2)
     
     
     

    Blank line between these.

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

    Again, I don't like using extra_args this way. We should definitely be using keyword args for this.

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

    no_renames=False over extra_args=[]

  5. rbtools/commands/__init__.py (Diff revision 2)
     
     

    Update this to the next point release for RBTools: 0.7.11

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

    Instead of checking the tool type, we should add a flag to SCMTool like supports_no_renames (defaulting to False) and set supports_no_renames = True in clients/git.py. Then we can check:

    if self.options.no_renames:
        if tool.supports_no_renames:
            # ...
        else:
            logging.error('The %s SCM tool does not support diffs without renames',
                          tool.type)
    
  7. rbtools/commands/diff.py (Diff revision 2)
     
     

    I'm not sure that we should be using extra_args for this. This seems like a job for a keyword argument.

  8. 
      
Brian LeBlanc
Barret Rennie
  1. Teeny tiny nitpicks :) You should probably get either David and/or Christian's feedback about this since we're adding new arguments to diff() and that could break third party SCM clients.

  2. rbtools/clients/git.py (Diff revisions 2 - 3)
     
     
     
     

    Can you split this up for one condition per line? It makes it easier to read.

  3. rbtools/commands/diff.py (Diff revisions 2 - 3)
     
     
     

    Blank line between these.

  4. 
      
Brian LeBlanc
Barret Rennie
  1. I'm happy with this as-is. Will wait for feedback from Chirstian and/or David before landing, however.

  2. 
      
David Trowbridge
  1. 
      
  2. rbtools/commands/diff.py (Diff revision 4)
     
     
     

    These can be combined into a single conditional.

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

    I suspect we want to make this a fatal error (i.e. raise CommandError instead of just log something).

  4. 
      
Brian LeBlanc
Review request changed

Change Summary:

Clean up structure around tool not supporting --no-renames flag based on feedback.

Commit:

-6aa90e72a8aaea957553e3c8492fa8ca2103358c
+276ea8299017aeeb582dd32a4d6e6b7e9200c257

Diff:

Revision 5 (+32 -14)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...