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

Review Request #9221 — Created Sept. 24, 2017 and submitted

Brian LeBlanc
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.


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

    Blank line between these.

  3. rbtools/clients/ (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/ (Diff revision 2)

    no_renames=False over extra_args=[]

  5. rbtools/commands/ (Diff revision 2)

    Update this to the next point release for RBTools: 0.7.11

  6. rbtools/commands/ (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/ Then we can check:

    if self.options.no_renames:
        if tool.supports_no_renames:
            # ...
            logging.error('The %s SCM tool does not support diffs without renames',
  7. rbtools/commands/ (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.

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/ (Diff revisions 2 - 3)

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

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

    Blank line between these.

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

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

    These can be combined into a single conditional.

  3. rbtools/commands/ (Diff revision 4)

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

Brian LeBlanc
Brian LeBlanc
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (8a66d1c)