Summary

rbtools commands improvements

Review Request #3726 — Created Jan. 6, 2013 and submitted Jan. 9, 2013, 6:20 p.m.

Information

Owner

smacleod

Repository

RBTools

Branch

api

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Has parent review request /r/3727

Add initial command help

'rbt help' can now be called to display a list of commands and usage
text. 'rbt help <command>' will output the help specific to a command
including the options it takes. Formatting and the  extensiveness of
help for each command will be improved in the future, but this is a
good start.

Add checking for number of command args

The number of args provided to the command is now checked against the
number the command accepts (based on the signature of its main
method). Usage text and an error are now displayed if the number of
arguments provided is invalid.

Add proper usage text to commands

Commands may now define 'args' which will be output with usage text to
show the arguments a command takes. Commands now have a 'usage' method
which will return the usage text. While this can be overridden, the
base class 'usage' should generally suffice.

Introduce a new class for defining options

A new 'Option' has been added which allows specifying a 'config_key'.
The provided 'config_key' will be used to retrieve a default value for
the option from the configuration files. This will take precedence
over the value provided for 'default'. This should also allow us to
provide the corresponding config file option when displaying help.

Additionally, all of the current commands have been switched over to
the new method for defining options, and the parsing code has been
changed accordingly.

Add missing imports and fix small bug

A couple imports were missing, and a object attribute was being
accessed without "self." prepended.

Change command author to The Review Board Project

A number of the commands built as part of the core rbtools were using
individual authors. In order to be clear these commands are developed
as part of the core project, the author has been changed.

Testing Done

Ran commands. Output help for commands. Checked commands with invalid number of arguments.

Issues

Description	From	Last Updated
Col: 13 E125 continuation line does not distinguish itself from next logical line	reviewbot	Jan. 6, 2013, 7:48 p.m.
Col: 23 E127 continuation line over-indented for visual indent	reviewbot	Jan. 6, 2013, 8 p.m.
Col: 13 E125 continuation line does not distinguish itself from next logical line	reviewbot	Jan. 6, 2013, 7:48 p.m.
Blank line between these.	chipx86	Jan. 6, 2013, 8:38 p.m.
"Review Board server". Should be fixed in other options across all files as well.	chipx86	Jan. 6, 2013, 8:44 p.m.
There's a faster one called cStringIO. Generally, you want to do: try: from cStringIO import StringIO except ImportError: from StringIO …	chipx86	Jan. 6, 2013, 8:36 p.m.
Should still go above, after the "from optparse", since it's a built-in Python module. Elsewhere, we do have a blank …	chipx86	Jan. 6, 2013, 8:57 p.m.
Blank line before this.	chipx86	Jan. 6, 2013, 8:34 p.m.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/main.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/patch.py
    rbtools/commands/post.py
    rbtools/commands/attach.py
    rbtools/commands/publish.py
    setup.py
    rbtools/commands/close.py
  Ignored Files:
    rbtools/api/serverinterface.py
    rbtools/api/settings.py
    rbtools/commands/rbinfo.py
    rbtools/commands/rbinfov2.py
    rbtools/knownissues.txt
    rbtools/commands/rbattach.py
    rbtools/commands/rbupload.py
    rbtools/commands/rb.py
    rbtools/commands/utils.py
    rbtools/commands/tests.py
    rbtools/commands/rbdiff.py
    rbtools/commands/rbcreate.py
    rbtools/commands/rbconfig.py
    rbtools/commands/rbpatch.py
    AUTHORS
    rbtools/commands/rbpublish.py
    rbtools/commands/resource-browser.py
    rbtools/commands/rbopen.py
    rbtools/commands/rbclose.py
    rbtools/commands/rbpost.py

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

The issue has been dropped. Show all issues

Col: 13
 E125 continuation line does not distinguish itself from next logical line

rbtools/commands/post.py (Diff revision 1)
The issue has been dropped. Show all issues
```
Col: 23
 E127 continuation line over-indented for visual indent
```

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

The issue has been dropped. Show all issues

Col: 13
 E125 continuation line does not distinguish itself from next logical line

Change Summary:

Removed the rename commits and moved them into a separate review request so only modifications are shown.

Description:

~		This work is on top of John Sintal's work on several commands. Once
	~	Has parent review request /r/3727
-		This has been reviewed, both will be pushed together.

		Add initial command help

		'rbt help' can now be called to display a list of commands and usage
		text. 'rbt help <command>' will output the help specific to a command
		including the options it takes. Formatting and the extensiveness of
		help for each command will be improved in the future, but this is a
		good start.

		Add checking for number of command args

		The number of args provided to the command is now checked against the
		number the command accepts (based on the signature of its main
		method). Usage text and an error are now displayed if the number of
		arguments provided is invalid.

		Add proper usage text to commands

		Commands may now define 'args' which will be output with usage text to
		show the arguments a command takes. Commands now have a 'usage' method
		which will return the usage text. While this can be overridden, the
		base class 'usage' should generally suffice.

		Introduce a new class for defining options

		A new 'Option' has been added which allows specifying a 'config_key'.
		The provided 'config_key' will be used to retrieve a default value for
		the option from the configuration files. This will take precedence
		over the value provided for 'default'. This should also allow us to
		provide the corresponding config file option when displaying help.

		Additionally, all of the current commands have been switched over to
		the new method for defining options, and the parsing code has been
		changed accordingly.

		Add missing imports and fix small bug

		A couple imports were missing, and a object attribute was being
		accessed without "self." prepended.

		Change command author to The Review Board Project

		A number of the commands built as part of the core rbtools were using
		individual authors. In order to be clear these commands are developed
		as part of the core project, the author has been changed.
-
-		Use "rbt" instead of "rb" for base command.
-
-		The base command "rb" conflicted with another package, so to avoid
-		problems we will now use "rbt" instead. I refactored things a bit to
-		make changing this name easier in the future.
-
-		Add Steven MacLeod and John Sintal to AUTHORS
-
-
-
-		Remove unused files.
-
-		This removes a number of unused command and api files which have been
-		sitting around. It's possible some of the logic present in the command
-		files could be useful in the future, but the implementation details
-		would need to be changed drastically.

Diff:

Revision 2 (+505 -351)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/main.py
	rbtools/commands/patch.py
	rbtools/commands/post.py
	rbtools/commands/publish.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/main.py
    rbtools/commands/publish.py
    rbtools/commands/close.py
    rbtools/commands/patch.py
    rbtools/commands/post.py
    rbtools/commands/attach.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
  Ignored Files:

rbtools/commands/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Blank line between these.
```

rbtools/commands/attach.py (Diff revision 2)

Food for thought for a future change. It'd be nice to have some generally standard options we can include, like these.

rbtools/commands/attach.py (Diff revision 2)

The issue has been resolved. Show all issues

"Review Board server".

Should be fixed in other options across all files as well.

rbtools/commands/main.py (Diff revision 2)

The issue has been resolved. Show all issues

There's a faster one called cStringIO. Generally, you want to do:

try:
    from cStringIO import StringIO
except ImportError:
    from StringIO import StringIO

rbtools/commands/main.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Blank line before this.
```

Change Summary:

Make changes from Christian's Review.

Diff:

Revision 3 (+511 -351)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/main.py
	rbtools/commands/patch.py
	rbtools/commands/post.py
	rbtools/commands/publish.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/main.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/patch.py
    rbtools/commands/post.py
    rbtools/commands/attach.py
    rbtools/commands/publish.py
    rbtools/commands/close.py
  Ignored Files:

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

The issue has been resolved. Show all issues

Should still go above, after the "from optparse", since it's a built-in Python module.

Elsewhere, we do have a blank line between the try/excepts and other imports. Sort of removes the 3 groups thing, but it's close enough.

Change Summary:

Update based on Christian's review.

Diff:

Revision 4 (+511 -351)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/main.py
	rbtools/commands/patch.py
	rbtools/commands/post.py
	rbtools/commands/publish.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/main.py
    rbtools/commands/__init__.py
    rbtools/commands/diff.py
    rbtools/commands/patch.py
    rbtools/commands/post.py
    rbtools/commands/attach.py
    rbtools/commands/publish.py
    rbtools/commands/close.py
  Ignored Files:

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Merged into api (2b12805a6b392e85f4a6e21424e0628736c1b405).