Summary

Refactor out common diff generation code

Review Request #3864 — Created Feb. 11, 2013 and submitted Feb. 12, 2013, 10:43 a.m.

Information

Owner

smacleod

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

chipx86

Description

Refactor out common diff generation code.

The post and diff commands were using seperate repeated code for
generating diffs. This common code is now a function which the
commands can call to generate the diff.

Testing Done

Generated a couple diffs using rbt diff, and posted this review request using rbt post.

Issues

Description	From	Last Updated
Extra blank lines.	chipx86	Feb. 12, 2013, 10:34 a.m.
Indented too far.	chipx86	Feb. 12, 2013, 10:34 a.m.
Col: 1 E302 expected 2 blank lines, found 0	reviewbot	Feb. 12, 2013, 10:36 a.m.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/post.py
    rbtools/utils/diffs.py
    rbtools/commands/diff.py
  Ignored Files:

rbtools/utils/diffs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Extra blank lines.
```
1. SM
  
  smacleod Feb. 12, 2013, 10:34 a.m.
  This was to stop the pep8 script from complaining, it got left in.

rbtools/utils/diffs.py (Diff revision 1)

The issue has been resolved. Show all issues

Indented too far.

rbtools/utils/diffs.py (Diff revision 1)

At some point we should figure out if there's a better way to do this. Having SVN-specific options here is funky.

smacleod Feb. 12, 2013, 10:34 a.m.

Yeah, after 0.5 is out, I'm going to take a serious look at the clients
code, and how we call into it.

Change Summary:

Updated based on Christian's review

Diff:

Revision 2 (+31 -28)

Show changes

	rbtools/commands/diff.py
	rbtools/commands/post.py
	rbtools/utils/diffs.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/post.py
    rbtools/utils/diffs.py
    rbtools/commands/diff.py
  Ignored Files:

rbtools/utils/diffs.py (Diff revision 2)

The issue has been dropped. Show all issues

Col: 1
 E302 expected 2 blank lines, found 0

smacleod Feb. 12, 2013, 10:37 a.m.

The pep8 script is just checking for two black lines above, instead of between.

Change Summary:: Removed review request dependency
Description:: Refactor out common diff generation code.

The post and diff commands were using seperate repeated code for
generating diffs. This common code is now a function which the
commands can call to generate the diff.
-
-
Depends on /r/3847

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (bec5ebe582aabcea089f6662b89553ce4ad04d1d).

		Refactor out common diff generation code.

		The post and diff commands were using seperate repeated code for
		generating diffs. This common code is now a function which the
		commands can call to generate the diff.
-
-		Depends on /r/3847