Summary

Enables the option for color to be displayed Open Issues and Ship It!

Review Request #7770 — Created Nov. 14, 2015 and discarded May 18, 2016, 2:36 p.m.

Information

Owner

danarteaga

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools, students

People

Description

RBT Status currently does not have the option for color to be displayed for Open Issues and Ship It! reviews.
I am adding color to make Open Issues yellow and Ship It! reviews green. This is a dependent feature of the RBTools color output.

Testing Done

I ran the ./reviewboard/manage.py test and they passed. I ran rbt status, and it displays yellow for Open Issues and green for Ship it!

Files

Issues

Description	From	Last Updated
I believe this should be categorized with the above import statements in alphabetical order. Most the files in rbtools I …	JW jwu	Nov. 16, 2015, 10:07 a.m.
Needs a space at end of line.	brennie	Nov. 20, 2015, 3:43 p.m.
Needs a ,.	brennie	Nov. 20, 2015, 3:43 p.m.
Needs a ,.	brennie	Nov. 20, 2015, 3:43 p.m.
This isn't the approach you're going to want to use. You're going to want to look at https://pypi.python.org/pypi/colorama and use, …	brennie	Nov. 21, 2015, 10:12 a.m.
See other comment.	brennie	Nov. 20, 2015, 3:39 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 20, 2015, 3:48 p.m.
Put the ] on the next line. This should have a , at the end.	brennie	Nov. 20, 2015, 3:43 p.m.
Col: 33 E127 continuation line over-indented for visual indent	reviewbot	Nov. 29, 2015, 5:11 p.m.
Col: 33 E127 continuation line over-indented for visual indent	reviewbot	Nov. 29, 2015, 5:11 p.m.
Don't need this here.	brennie	Nov. 29, 2015, 4:43 p.m.
Remove this.	brennie	Nov. 29, 2015, 4:42 p.m.
You need to do add Fore.RESET to the end of these strings.	brennie	Nov. 29, 2015, 5:10 p.m.
This should be in your other patch. You committed on the wrong branch. Ping me early next week and we …	brennie	Nov. 28, 2015, 9:07 p.m.
Undo this change.	brennie	Nov. 28, 2015, 9:08 p.m.
Undo this	brennie	Nov. 28, 2015, 9:08 p.m.
This shouldn't be in this patch.	brennie	Dec. 1, 2015, 7:31 p.m.
This shouldn't be in your patch. Make sure you are posting correctly with rbt post -u branch1..branch2	brennie	Dec. 1, 2015, 7:31 p.m.
Undo this.	brennie	Dec. 1, 2015, 7:31 p.m.
Remove this.	brennie	Dec. 1, 2015, 7:42 p.m.
Can you format this as: status = ('%sOpen Issues (%s)%s' % (Fore.YELLOW, request.issue_open_count, Fore.RESET)) with appropriate wrapping?	brennie	Dec. 1, 2015, 7:42 p.m.
Likewise here.	brennie	Dec. 1, 2015, 7:42 p.m.
'ColoredFormatter' imported but unused	reviewbot	Dec. 1, 2015, 7:45 p.m.
This should be renamed to status_width and the result is no longer always len(status). Becuase status contains control codes, which …	brennie	Dec. 2, 2015, 5:01 p.m.

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    setup.py
    rbtools/commands/__init__.py



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

rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```

Testing Done:

None, want to see if I'm on the right track

Groups:

+	students

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

The issue has been resolved. Show all issues

I believe this should be categorized with the above import statements in alphabetical order. Most the files in rbtools I looked at seem to just have all the imports from six on their own.

brennie Nov. 16, 2015, 9:53 a.m.

Actually, this should stay where it is.
RBTools uses very little in the way of 3-rd party dependencies and for the most part is just six. colorlog is also a 3-rd party dependency so we should keep it where it is.
Imports are organized as:

from __future__ import unicode_literals  # Always first.

# Standard library imports
from os import whatever

# 3-rd party imports
from some_lib import whatever

# Project imports
from rbtools import whatever

It appears you ahve posted two changes at once.

Can you run rbt post -r 7770 rbtcolors..dev/colorize-status (or whatever your branch names are).

That way you will only have the latter code in this patch.

You should also mark this as depending on your previous work.

rbtools/commands/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Needs a space at end of line.
```
rbtools/commands/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Needs a ,.
```
rbtools/commands/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Needs a ,.
```

setup.py (Diff revision 1)

The issue has been resolved. Show all issues

Put the ] on the next line. This should have a , at the end.

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

The issue has been resolved. Show all issues

This isn't the approach you're going to want to use. You're going to want to look at https://pypi.python.org/pypi/colorama and use, e.g., Fore.YELLOW for open issues.

You don't want to do this with

rbtools/commands/status.py (Diff revision 1)
The issue has been resolved. Show all issues
```
See other comment.
```

Description:

		RBT Status currently does not have the option for color to be displayed for Open Issues and Ship It! reviews.
~		I am adding color to make Open Issues yellow and Ship It! reviews green.
	~	I am adding color to make Open Issues yellow and Ship It! reviews green. This is a dependent feature of the RBTools color output.

Testing Done:

~		None, want to see if I'm on the right track
	~	I ran the ./reviewboard/manage.py test and they passed, but I received an error when I ran the rbt status command. I've included a picture of what it looks like.

Diff:

Revision 2 (+26 -10)

Show changes

	setup.py
	rbtools/commands/__init__.py
	rbtools/commands/status.py

Added Files:

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    setup.py
    rbtools/commands/__init__.py



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

rbtools/commands/status.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 33
 E127 continuation line over-indented for visual indent
```
rbtools/commands/status.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 33
 E127 continuation line over-indented for visual indent
```

Testing Done:

~		I ran the ./reviewboard/manage.py test and they passed, but I received an error when I ran the rbt status command. I've included a picture of what it looks like.
	~	I ran the ./reviewboard/manage.py test and they passed. I ran rbt status, and it displays yellow for Open Issues.

Diff:

Revision 3 (+26 -10)

Show changes

	setup.py
	rbtools/commands/__init__.py
	rbtools/commands/status.py

Added Files:

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    setup.py
    rbtools/commands/__init__.py



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

rbtools/commands/status.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Don't need this here.
```

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

The issue has been resolved. Show all issues

Remove this.

rbtools/commands/status.py (Diff revision 3)
The issue has been resolved. Show all issues
```
You need to do add Fore.RESET to the end of these strings.
```

setup.py (Diff revision 3)

The issue has been resolved. Show all issues

This should be in your other patch. You committed on the wrong branch. Ping me early next week and we can talk about rebasing.

Diff:

Revision 4 (+26 -11)

Show changes

	setup.py
	rbtools/commands/__init__.py
	rbtools/commands/status.py

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    setup.py
    rbtools/commands/__init__.py



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

rbtools/commands/__init__.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Undo this change.
```
setup.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Undo this
```

Diff:

Revision 5 (+15 -11)

Show changes

	setup.py
	rbtools/commands/__init__.py
	rbtools/commands/status.py

Tool: Pyflakes
Processed Files:
    rbtools/commands/status.py
    setup.py
    rbtools/commands/__init__.py



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

Let's touch base and we can go over fixing up your git issues.

rbtools/commands/__init__.py (Diff revision 5)
The issue has been dropped. Show all issues
```
This shouldn't be in this patch.
```

setup.py (Diff revision 5)

The issue has been dropped. Show all issues

This shouldn't be in your patch.

Make sure you are posting correctly with rbt post -u branch1..branch2

setup.py (Diff revision 5)
The issue has been dropped. Show all issues
```
Undo this.
```

Diff:

Revision 6 (+20 -4)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/status.py

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



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

Diff:

Revision 7 (+22 -4)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/status.py

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



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

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

The issue has been resolved. Show all issues

Remove this.

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

The issue has been resolved. Show all issues

Can you format this as:


status = ('%sOpen Issues (%s)%s'
          % (Fore.YELLOW, request.issue_open_count, Fore.RESET))


with appropriate wrapping?

rbtools/commands/status.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Likewise here.
```

Summary:

-	Enables the option for color to be displayed Open Issues and Ship It! [WIP]
+	Enables the option for color to be displayed Open Issues and Ship It!

Testing Done:

~		I ran the ./reviewboard/manage.py test and they passed. I ran rbt status, and it displays yellow for Open Issues.
	~	I ran the ./reviewboard/manage.py test and they passed. I ran rbt status, and it displays yellow for Open Issues and green for Ship it!

Diff:

Revision 8 (+10 -4)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/status.py

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



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

rbtools/commands/status.py (Diff revision 8)
The issue has been resolved. Show all issues
```
 'ColoredFormatter' imported but unused
```

Diff:

Revision 9 (+9 -4)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/status.py

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



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

Color for Open Issues (Revision 1)
```
Can you update this screenshot?
```
1. DA
  
  danarteaga Dec. 1, 2015, 8:05 p.m.
  Just did!

Added Files:

Fix it!

This regresses the table-formatted output:


(rb)~/W/R/s/reviewboard (dev/refactor-registry/fields) $ rbt status
=====================================================================================
Status                   Review Request
=====================================================================================
Ship It! (1)   r/7787 - Do not attempt to render uninstantiated fields
Pending                  r/7786 - Use registries for hosting services
Pending                  r/7785 - Use registries for authentication backends


The r/7787 bit should be under the Review Request column.

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

An issue was opened. Show all issues

This should be renamed to status_width and the result is no longer always len(status).

Becuase status contains control codes, which are zero-width characters, this will end up calculating the wrong column width. You will have to calculate the width in the cases above and pass it through here.

If you do rename this status_width, you'll have to update whatever takes in these to print the table to expect status_width instead of status_len.

Review Board 7.0 alpha 0 (dev)

Enables the option for color to be displayed Open Issues and Ship It!

Information

Reviewers

Testing Done:

Groups:

Description:

Testing Done:

Diff:

Added Files:

Testing Done:

Diff:

Added Files:

Diff:

Diff:

Diff:

Diff:

Summary:

Testing Done:

Diff:

Diff:

Added Files:

Status: Discarded