RB Diff

Review Request #3429 — Created Oct. 18, 2012 and submitted

Information

RBTools
master

Reviewers

RB command that displays a diff in the terminal.
Git: Tried out --tracking-branch, --parent and no args, all of which produced diff properly
SVN: Tried out without args, and listing individual file(s), all of which produced diff(s) properly.

Description From Last Updated

Unused, should be removed.

SM smacleod

Unused imports, should be removed.

SM smacleod

These imports aren't used and should be removed.

SM smacleod

These will be unused after the code is removed, and should be removed as well.

SM smacleod

Unused, should be removed.

SM smacleod

These options aren't needed for the diff command.

SM smacleod

These options aren't needed for the diff command.

SM smacleod

This options isn't needed for the diff command.

SM smacleod

You'll want to remove the defaults from here which don't make sense after deleting them from above.

SM smacleod

These defaults should probably stay.

SM smacleod

This default should stay

SM smacleod

I'm not sure what's going on with the indentation here. This shouldn't be indented so much.

SM smacleod

This code isn't necessary for the diff command.

SM smacleod

Blank line between these.

chipx86chipx86

Are these needed?

chipx86chipx86

Doc strings must always be sentences.

chipx86chipx86

Alphabetical.

chipx86chipx86
SM
  1. 
      
  2. rbtools/commands/diff.py (Diff revision 1)
     
     
    Show all issues
    Unused, should be removed.
  3. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
    Show all issues
    Unused imports, should be removed.
  4. rbtools/commands/diff.py (Diff revision 1)
     
     
     
    Show all issues
    These imports aren't used and should be removed.
  5. rbtools/commands/diff.py (Diff revision 1)
     
     
     
    Show all issues
    These will be unused after the code is removed, and should be removed as well.
  6. rbtools/commands/diff.py (Diff revision 1)
     
     
    Show all issues
    Unused, should be removed.
  7. rbtools/commands/diff.py (Diff revision 1)
     
     
    Thanks for including my name, but you're welcome to take full credit here. It's your project, and I think you should be able to claim authorship. Feel free to switch this to "John Sintal" If you'd like.
  8. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues
    These options aren't needed for the diff command.
  9. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues
    These options aren't needed for the diff command.
  10. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
    Show all issues
    This options isn't needed for the diff command.
  11. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues
    You'll want to remove the defaults from here which don't make sense after deleting them from above.
  12. rbtools/commands/diff.py (Diff revision 1)
     
     
    Show all issues
    I'm not sure what's going on with the indentation here. This shouldn't be indented so much.
  13. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
    Show all issues
    This code isn't necessary for the diff command.
  14. 
      
JS
JS
JS
RO
  1. ABC
  2. 
      
SM
  1. It's fine to delete the current 'rbdiff.py' and rename yours. If needed we can look at the history to get its contents.
  2. rbtools/commands/diff.py (Diff revisions 1 - 2)
     
     
     
     
    Show all issues
    These defaults should probably stay.
  3. rbtools/commands/diff.py (Diff revisions 1 - 2)
     
     
    Show all issues
    This default should stay
  4. 
      
JS
JS
JS
JS
JS
JS
chipx86
  1. Looking good.
  2. rbtools/commands/rbdiff.py (Diff revision 7)
     
     
     
    Show all issues
    Blank line between these.
  3. rbtools/commands/rbdiff.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues
    Are these needed?
  4. rbtools/commands/rbdiff.py (Diff revision 7)
     
     
    Show all issues
    Doc strings must always be sentences.
  5. setup.py (Diff revision 7)
     
     
     
    Show all issues
    Alphabetical.
  6. 
      
mike_conley
  1. Updated patch?
  2. 
      
JS
david
  1. 
      
  2. rbtools/commands/rbdiff.py (Diff revision 8)
     
     
    I think your error would be fixed if you did tool.diff(list(args))
  3. 
      
JS
JS
JS
SM
  1. Ship It!
  2. 
      
JS
Review request changed
Status:
Completed
Change Summary:
Pushed to api (89797552573f23bbb6cc30e43afc391c4fb217dd).