Summary

Add local branch information to rbt status

Review Request #7951 — Created Feb. 7, 2016 and submitted April 4, 2016, 10:48 p.m.

Information

Owner

evanhunzinger

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Modified rbt post to append current local branch to the POST request, modified the rbt status to obtain the saved local branch from the GET request, and altered the formatting of rbt status to show the last local branch that was used to post the review request.

Testing Done

I tested both rbt post and rbt status manually.

`rbt post`

Create branches in both git and bzr repos and do rbt post and ensure that local_branch is added to the review request as extra_data
Create bookmark in hg repo and do rbt post and ensure that local_bookmark is added to the review request as extra_data
Do an rbt post in and hg repo without specifying a bookmark and ensure that no local_bookmark is not added to the review request as extra data
Do an rbt post with an SCM that doesn't have branching/bookmarking capabilities (I used svn) and ensure no unintended extra_data is being added

`rbt status`

Run rbt status in repos that support branching (both git and bzr) and ensure that only the 'Branch' column shows up
Run rbt status in repos that support branching (hg) and ensure that only the 'Bookmark' column shows up
Run rbt status in a repo that don't support branching/bookmarking (I used svn) and ensure that neither 'Branch' nor 'Bookmark' columns show up
Run rbt status --all and ensure that both the 'Branch' and 'Bookmark' columns show up, and also that each branch and bookmark shows up properly associated with the correct review request
Make terminal large enough to fit each record of the rbt status output in one line and ensure that it outputs properly
Shrink terminal small enough to require word wraps for various records in the rbt status output and ensure that it outputs properly

Files

Issues

Description	From	Last Updated
I think we should print a blank string instead of "None"	david	April 1, 2016, 9:48 p.m.
I wouldn't mind other opinions on this, it looks like TextTable will try to limit the size of the table …	EV evanhunzinger	April 2, 2016, 12:30 p.m.
This should be extra_data.local_branch.	brennie	Feb. 8, 2016, 3:07 p.m.
Col: 30 E128 continuation line under-indented for visual indent	reviewbot	Feb. 14, 2016, 11:09 a.m.
Col: 30 W503 line break before binary operator	reviewbot	Feb. 14, 2016, 11:09 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 14, 2016, 11:09 a.m.
Col: 30 E128 continuation line under-indented for visual indent	reviewbot	Feb. 14, 2016, 11:09 a.m.
Col: 30 W503 line break before binary operator	reviewbot	Feb. 14, 2016, 11:09 a.m.
Col: 80 E501 line too long (121 > 79 characters)	reviewbot	Feb. 14, 2016, 11:09 a.m.
This should just be None, in my opinion.	brennie	Feb. 8, 2016, 3:43 p.m.
This should just be None, in my opinion.	brennie	Feb. 14, 2016, 11:09 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Feb. 14, 2016, 12:27 p.m.
Col: 30 E113 unexpected indentation	reviewbot	Feb. 14, 2016, 12:32 p.m.
Col: 30 E111 indentation is not a multiple of four	reviewbot	Feb. 14, 2016, 12:32 p.m.
Col: 30 E111 indentation is not a multiple of four	reviewbot	Feb. 14, 2016, 12:32 p.m.
This should just be blank, actually. That way, if a user did have an actual None branch, it wouldnt be …	brennie	Feb. 21, 2016, 12:01 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Feb. 21, 2016, 12:14 p.m.
Col: 80 E501 line too long (102 > 79 characters)	reviewbot	Feb. 21, 2016, 12:14 p.m.
Col: 80 E501 line too long (99 > 79 characters)	reviewbot	Feb. 21, 2016, 12:14 p.m.
We're not consistent (especially in old code like this file), but docstrings should be in the imperative mood rather than …	david	March 6, 2016, 11:36 a.m.
This is getting pretty ugly and repetitive. Can we abstract this somehow so we store the table and then print …	david	March 6, 2016, 11:36 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	March 6, 2016, 11:49 a.m.
I wasn't sure about this solution, should I be utilizing the SCMClient Capabilities object instead?	EV evanhunzinger	March 6, 2016, 12:06 p.m.
Col: 17 W503 line break before binary operator	reviewbot	March 6, 2016, 11:49 a.m.
Col: 17 W503 line break before binary operator	reviewbot	March 6, 2016, 11:49 a.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	March 6, 2016, 11:49 a.m.
Pretty close. Formatting should be: Returns: bytes: A string with ...	david	March 19, 2016, 12:36 p.m.
Same here with the "type"	david	March 19, 2016, 12:36 p.m.
This is a third party import It should be in its own group. Also, you'll have to update setup.py to …	brennie	March 26, 2016, 12:56 p.m.
Hmm. What happens in the case where a user does --all with both git and hg repos? In that case …	david	March 26, 2016, 12:52 p.m.
This doesn't handle the can_bookmark case. Perhaps we should just have if 'branch' in request: ?	david	March 26, 2016, 12:52 p.m.
Dedent this 4 spaces ('local_bookmark' should start on the same column as self.tool.can_bookmark)	david	March 19, 2016, 12:37 p.m.
Dedent this 2 spaces.	david	March 19, 2016, 12:37 p.m.
The len fields are no longer required, right?	david	March 19, 2016, 12:37 p.m.
This is no longer required, right?	david	March 19, 2016, 12:38 p.m.
Missing trailing comma.	brennie	March 31, 2016, 11:29 p.m.
This can be simplified a bit. We don't need to have the not has_* checks in there (we can always …	david	April 1, 2016, 9:47 p.m.
If you reorder this a bit, it becomes simpler: request_stats = { 'status': status, 'summary': 'r/%s - %s' % (request.id, …	david	April 1, 2016, 9:47 p.m.
Col: 68 E502 the backslash is redundant between brackets	reviewbot	April 2, 2016, 12:30 p.m.
Col: 59 E703 statement ends with a semicolon	reviewbot	April 2, 2016, 12:30 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	April 2, 2016, 12:30 p.m.
This should be grouped with the other 3rd party imports (the texttable one)	david	April 3, 2016, 12:12 p.m.
Is it possible to skip all this and just pass in the max width to the Texttable constructor?	david	April 3, 2016, 12:12 p.m.
Please keep this is alphabetical order.	david	April 3, 2016, 12:12 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	April 3, 2016, 12:12 p.m.

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/post.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/status.py
    rbtools/commands/post.py

rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 30
 E128 continuation line under-indented for visual indent
```
rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 30
 W503 line break before binary operator
```
rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 30
 E128 continuation line under-indented for visual indent
```
rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 30
 W503 line break before binary operator
```
rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (121 > 79 characters)
```

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

The issue has been dropped. Show all issues

This should be extra_data.local_branch.

brennie Feb. 8, 2016, 3:08 p.m.

Actually, I forgot that extra_data__ gets converted to extra_data. in request methods, so this is fine.

rbtools/commands/status.py (Diff revision 1)
The issue has been dropped. Show all issues
```
This should just be None, in my opinion.
```
1. brennie Feb. 8, 2016, 3:43 p.m.
  RB filed two issues at once. This must be a bug :(
rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should just be None, in my opinion.
```

Change Summary:

Fixed formatting issues addressed by Review Bot as well as suggested change from Barret.

Diff:

	rbtools/clients/bazaar.py
	rbtools/commands/post.py
	rbtools/commands/status.py

	rbtools/clients/bazaar.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/commands/post.py
	rbtools/commands/status.py

	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/commands/post.py
	rbtools/commands/status.py

	setup.py
	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/commands/post.py
	rbtools/commands/status.py

	+	I tested both `rbt post` and `rbt status` manually.
	+
	+	`rbt post`
	+
	+	Create branches in both git and bzr repos and do `rbt post` and ensure that `local_branch` is added to the review request as `extra_data`
	+	Create bookmark in hg repo and do `rbt post` and ensure that `local_bookmark` is added to the review request as `extra_data`
	+	Do an `rbt post` in and hg repo without specifying a bookmark and ensure that no `local_bookmark` is not added to the review request as extra data
	+	Do an `rbt post` with an SCM that doesn't have branching/bookmarking capabilities (I used svn) and ensure no unintended `extra_data` is being added
	+
	+	`rbt status`
	+
	+	Run `rbt status` in repos that support branching (both git and bzr) and ensure that only the 'Branch' column shows up
	+	Run `rbt status` in repos that support branching (hg) and ensure that only the 'Bookmark' column shows up
	+	Run `rbt status` in a repo that don't support branching/bookmarking (I used svn) and ensure that neither 'Branch' nor 'Bookmark' columns show up
	+	Run `rbt status --all` and ensure that both the 'Branch' and 'Bookmark' columns show up, and also that each branch and bookmark shows up properly associated with the correct review request
	+	Make terminal large enough to fit each record of the `rbt status` output in one line and ensure that it outputs properly
	+	Shrink terminal small enough to require word wraps for various records in the `rbt status` output and ensure that it outputs properly