Summary

RB Diff

Review Request #3429 — Created Oct. 18, 2012 and submitted Jan. 9, 2013, 6:18 p.m.

Information

Owner

jsintal

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

RB command that displays a diff in the terminal.

Testing Done

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.

Issues

Description	From	Last Updated
Unused, should be removed.	SM smacleod	Oct. 19, 2012, 1:01 p.m.
Unused imports, should be removed.	SM smacleod	Oct. 19, 2012, 1:01 p.m.
These imports aren't used and should be removed.	SM smacleod	Oct. 19, 2012, 1:02 p.m.
These will be unused after the code is removed, and should be removed as well.	SM smacleod	Oct. 19, 2012, 1:02 p.m.
Unused, should be removed.	SM smacleod	Oct. 19, 2012, 1:04 p.m.
These options aren't needed for the diff command.	SM smacleod	Oct. 19, 2012, 12:57 p.m.
These options aren't needed for the diff command.	SM smacleod	Oct. 19, 2012, 12:57 p.m.
This options isn't needed for the diff command.	SM smacleod	Oct. 19, 2012, 12:57 p.m.
You'll want to remove the defaults from here which don't make sense after deleting them from above.	SM smacleod	Oct. 19, 2012, 12:57 p.m.
These defaults should probably stay.	SM smacleod	Oct. 23, 2012, 5:39 p.m.
This default should stay	SM smacleod	Oct. 23, 2012, 5:39 p.m.
I'm not sure what's going on with the indentation here. This shouldn't be indented so much.	SM smacleod	Oct. 19, 2012, 1:05 p.m.
This code isn't necessary for the diff command.	SM smacleod	Oct. 19, 2012, 1:05 p.m.
Blank line between these.	chipx86	Dec. 4, 2012, 3:50 p.m.
Are these needed?	chipx86	Dec. 4, 2012, 3:51 p.m.
Doc strings must always be sentences.	chipx86	Dec. 4, 2012, 3:51 p.m.
Alphabetical.	chipx86	Dec. 4, 2012, 3:52 p.m.

rbtools/commands/diff.py (Diff revision 1)
Show all issues
```
Unused, should be removed.
```
rbtools/commands/diff.py (Diff revision 1)
Show all issues
```
Unused imports, should be removed.
```

rbtools/commands/diff.py (Diff revision 1)

Show all issues

These imports aren't used and should be removed.

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.

rbtools/commands/diff.py (Diff revision 1)
Show all issues
```
Unused, should be removed.
```

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.

rbtools/commands/diff.py (Diff revision 1)

Show all issues

These options aren't needed for the diff command.

rbtools/commands/diff.py (Diff revision 1)

Show all issues

These options aren't needed for the diff command.

rbtools/commands/diff.py (Diff revision 1)

Show all issues

This options isn't needed for the diff command.

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.

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.

rbtools/commands/diff.py (Diff revision 1)

Show all issues

This code isn't necessary for the diff command.

Description:

		WIP

		This is a simple RB command that displays a diff in the terminal.

~		Looking for feedback. This is essential Steven's rb post but stripped down from the unnecessary stuff. I also need to add support for tracking-branch.
	~	Looking for feedback. This was essentially Steven's rb post but stripped down from the unnecessary stuff. I also need to add support for tracking-branch.

Diff:

Revision 2 (+110)

Show changes

	setup.py
	rbtools/commands/diff.py

Description:

~		WIP
	~	[WIP]

		This is a simple RB command that displays a diff in the terminal.

~		Looking for feedback. This was essentially Steven's rb post but stripped down from the unnecessary stuff. I also need to add support for tracking-branch.
	~	TO DO:
	+	- Write tests that test all arguments
	+	- Remove the unneeded options which depending on lower level legacy code. This will be tricky and will need the consultation of the other mentors.

Testing Done:

	+	Tried out --tracking-branch and --parent, which seem to work well.
	+	Having difficulty finding a clear way to test this command.

Change Summary:

Updated to do list. That's it.

Description:

		[WIP]

		This is a simple RB command that displays a diff in the terminal.

		TO DO:
		- Write tests that test all arguments
~		- Remove the unneeded options which depending on lower level legacy code. This will be tricky and will need the consultation of the other mentors.
	~	- Remove the unneeded options which depending on lower level legacy code. This will be tricky and will need the consultation of the other mentors.
	+	- Rename the file to 'rbdiff.py' for consistency once I can confirm to deleted the rbdiff.py that's already in the repo.

```
ABC
```

It's fine to delete the current 'rbdiff.py' and rename yours. If needed we can look at the history to get its contents.

rbtools/commands/diff.py (Diff revisions 1 - 2)
Show all issues
```
These defaults should probably stay.
```
rbtools/commands/diff.py (Diff revisions 1 - 2)
Show all issues
```
This default should stay
```

Diff:

Revision 3 (+118 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

Summary:

-	[WIP] rb diff - prints a diff to terminal
+	RB Diff

Change Summary:

Rename the rbdiff command in setup.py

Description:

		[WIP]

		This is a simple RB command that displays a diff in the terminal.

		TO DO:
		- Write tests that test all arguments
~		- Remove the unneeded options which depending on lower level legacy code. This will be tricky and will need the consultation of the other mentors.
	~	- Remove the unneeded options which depending on lower level legacy code. This will be tricky and will need the consultation of the other mentors.
-		- Rename the file to 'rbdiff.py' for consistency once I can confirm to deleted the rbdiff.py that's already in the repo.

Diff:

Revision 4 (+118 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

Change Summary:

Changed the way commands are branched from other features (rb-general). Ex.: http://pastie.org/5468764

Diff:

Revision 5 (+108 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

Change Summary:

rebased on rb-general

Description:

~		[WIP]
	~	RB command that displays a diff in the terminal.
-
-		This is a simple RB command that displays a diff in the terminal.

		TO DO:
~		- Write tests that test all arguments
	~	- SVN Error, am I using missing args to 'rb diff' when using SVN? http://pastie.org/5477544
-		- Remove the unneeded options which depending on lower level legacy code. This will be tricky and will need the consultation of the other mentors.

Testing Done:

~		Tried out --tracking-branch and --parent, which seem to work well.
~		Having difficulty finding a clear way to test this command.
	~	Git
	~	Tried out --tracking-branch and --parent, produces diff properly

Diff:

Revision 6 (+90 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

Change Summary:

Rebased on using  the gettatr(options, field, default method for ALL

Diff:

Revision 7 (+90 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

```
Looking good.
```
rbtools/commands/rbdiff.py (Diff revision 7)
Show all issues
```
Blank line between these.
```

rbtools/commands/rbdiff.py (Diff revision 7)

Show all issues

Are these needed?

rbtools/commands/rbdiff.py (Diff revision 7)
Show all issues
```
Doc strings must always be sentences.
```
setup.py (Diff revision 7)
Show all issues
```
Alphabetical.
```

```
Updated patch?
```

Change Summary:

Addressed all issues

Diff:

Revision 8 (+84 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

rbtools/commands/rbdiff.py (Diff revision 8)

I think your error would be fixed if you did tool.diff(list(args))

Change Summary:

svn bug addressed

Diff:

Revision 9 (+84 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

Change Summary:

svn diff bug fixed

Testing Done:

~		Git
~		Tried out --tracking-branch and --parent, produces diff properly
	~	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.

Diff:

Revision 10 (+84 -196)

Show changes

	setup.py
	rbtools/commands/rbdiff.py

Description:

		RB command that displays a diff in the terminal.
-
-		TO DO:
-		- SVN Error, am I using missing args to 'rb diff' when using SVN? http://pastie.org/5477544

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to api (89797552573f23bbb6cc30e43afc391c4fb217dd).