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)
     
     
    Unused, should be removed.
  3. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
    Unused imports, should be removed.
  4. rbtools/commands/diff.py (Diff revision 1)
     
     
     
    These imports aren't used and should be removed.
  5. rbtools/commands/diff.py (Diff revision 1)
     
     
     
    These will be unused after the code is removed, and should be removed as well.
  6. rbtools/commands/diff.py (Diff revision 1)
     
     
    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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    These options aren't needed for the diff command.
  9. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    These options aren't needed for the diff command.
  10. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
    This options isn't needed for the diff command.
  11. rbtools/commands/diff.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    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)
     
     
    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)
     
     
     
     
    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)
     
     
     
     
    These defaults should probably stay.
  3. rbtools/commands/diff.py (Diff revisions 1 - 2)
     
     
    This default should stay
  4. 
      
JS
JS
JS
JS
JS
JS
chipx86
  1. Looking good.
  2. rbtools/commands/rbdiff.py (Diff revision 7)
     
     
     
    Blank line between these.
  3. rbtools/commands/rbdiff.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
    Are these needed?
  4. rbtools/commands/rbdiff.py (Diff revision 7)
     
     
    Doc strings must always be sentences.
  5. setup.py (Diff revision 7)
     
     
     
    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: Closed (submitted)

Change Summary:

Pushed to api (89797552573f23bbb6cc30e43afc391c4fb217dd).
Loading...