Summary

Introduce the status command

Review Request #3865 — Created Feb. 11, 2013 and submitted Feb. 24, 2013, 2:21 p.m.

Information

Owner

smacleod

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

chipx86

Description

Introduce the status command

The status command will display the id and summary of each pending
review request a user has.

Also, the get_root method has been renamed get_api and will now return the
RBClient instance along with the root resource. This allows commands
to receive direct access to the API client which can be useful if they
need to update login information after instantiating the client.

A few of the commands have also been updated to make less requests to
the Review Board server by taking advantage of the uri templates on
the root resource.

Testing Done

Ran commands, used rbt status to check review requests.

Issues

Description	From	Last Updated
Can we add a docstring for this? Also, assuming this is a private function, we should provide an underscore (and …	chipx86	Feb. 12, 2013, 11:58 a.m.
Might as well include self.get_cookie() right here.	chipx86	Feb. 12, 2013, 10:45 a.m.
Should this be the same thing? Can we bail out any other way than StopIteration?	chipx86	Feb. 12, 2013, 11:59 a.m.
Blank line between these.	chipx86	Feb. 12, 2013, noon
This seems like something worth having in a common place.	chipx86	Feb. 12, 2013, 12:12 p.m.
Should probably be a period before and after "Please login"	chipx86	Feb. 12, 2013, 12:12 p.m.
Does this get outputted in non-debug situations? If so, we should make sure it wraps at 80.	chipx86	Feb. 18, 2013, 6:27 p.m.
Excess blank line.	chipx86	Feb. 12, 2013, 12:13 p.m.
"Review Board"	chipx86	Feb. 18, 2013, 6:12 p.m.
"auth_required"	chipx86	Feb. 24, 2013, 2:20 p.m.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/status.py
    setup.py
    rbtools/commands/__init__.py
  Ignored Files:

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

The issue has been resolved. Show all issues

Can we add a docstring for this?

Also, assuming this is a private function, we should provide an underscore (and stick it at the end of the class).

Should this be _make_api_client? Calling it more than once would give different results.

rbtools/commands/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Might as well include self.get_cookie() right here.
```

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

The issue has been resolved. Show all issues

Should this be the same thing? Can we bail out any other way than StopIteration?

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

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

The issue has been resolved. Show all issues

This seems like something worth having in a common place.

rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should probably be a period before and after "Please login"
```

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

The issue has been resolved. Show all issues

Does this get outputted in non-debug situations? If so, we should make sure it wraps at 80.

smacleod Feb. 12, 2013, 12:23 p.m.

This will be output in non-debug situations. 

How do you suggest we take care of wrapping things like this? Should
we create a logging formatter which will take care of it automatically?

chipx86 Feb. 16, 2013, 8:37 p.m.

Python has a textwrap module of some sort that would work (rb-site uses this), but for now I'd say just figure out where the natural breaks are for 80 columns and hand-wrap.

smacleod Feb. 17, 2013, 10:53 a.m.

I was more referring to wrapping with respect to calls with the logging
module. (i.e. How can we make logging.warning(...) automatically wrap
what's printed to the screen.)

chipx86 Feb. 17, 2013, 10:56 a.m.

Right. I don't know the answer to that. I think we'd probably need our own ConsoleHandler or whatever that put everything through the textwrap module.

rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Excess blank line.
```

Change Summary:

Updated based on Christian's review

Description:

		Introduce the status command

		The status command will display the id and summary of each pending
		review request a user has.

~		Also refactor RBClient instantiation out of get_root.
	~	Also, the get_root method has been renamed get_api and will now return the
	+	RBClient instance along with the root resource. This allows commands
	+	to receive direct access to the API client which can be useful if they
	+	need to update login information after instantiating the client.

~		A new get_api_client method has been created to take care of RBClient
~		instantion. The get_root method will now optionally accept an RBClient
~		instance in the api_client parameter. This allows commands to receive
	~	A few of the commands have also been updated to make less requests to
	~	the Review Board server by taking advantage of the uri templates on
	~	the root resource.
-		direct access to the API client which can be useful if they need to
-		update login information after instantiating the client.

Testing Done:

Ran commands, used rbt status to check review requests.

Diff:

Revision 2 (+208 -63)

Show changes

	setup.py
	rbtools/api/client.py
	rbtools/commands/__init__.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/patch.py
	rbtools/commands/post.py
	rbtools/commands/publish.py
	2 more

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/status.py
    rbtools/api/client.py
    rbtools/commands/__init__.py
    rbtools/commands/close.py
    rbtools/commands/patch.py
    rbtools/commands/post.py
    rbtools/commands/attach.py
    rbtools/commands/publish.py
    setup.py
    rbtools/utils/users.py
  Ignored Files:

rbtools/commands/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"Review Board"
```

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

Can this be done in a common place so that all commands can get it for free?

Change Summary:

Updated based on Christian's reviews.

Diff:

Revision 3 (+213 -65)

Show changes

	setup.py
	rbtools/api/client.py
	rbtools/commands/__init__.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/patch.py
	rbtools/commands/post.py
	rbtools/commands/publish.py
	2 more

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    rbtools/commands/status.py
    rbtools/api/client.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
    setup.py
    rbtools/utils/users.py
  Ignored Files:

Ship it!

```
Found a typo. After that, ship it!
```
rbtools/utils/users.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"auth_required"
```

rbtools/utils/users.py (Diff revision 3)

Eventually we should find a way to handle this without assuming raw input, but that's for another day.

Status: Closed (submitted)

Change Summary:

Pushed to master (fea88b90fa27cd4a841cac502430551d25968d92, 8709d10a5c8dbb6bb5effc79038b96e800b305a8).