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

Review Request #7770 — Created Nov. 14, 2015 and discarded

Information

RBTools
master

Reviewers

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.

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!


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

Needs a space at end of line.

brenniebrennie

Needs a ,.

brenniebrennie

Needs a ,.

brenniebrennie

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, …

brenniebrennie

See other comment.

brenniebrennie

Col: 80 E501 line too long (84 > 79 characters)

reviewbotreviewbot

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

brenniebrennie

Col: 33 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 33 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Don't need this here.

brenniebrennie

Remove this.

brenniebrennie

You need to do add Fore.RESET to the end of these strings.

brenniebrennie

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

brenniebrennie

Undo this change.

brenniebrennie

Undo this

brenniebrennie

This shouldn't be in this patch.

brenniebrennie

This shouldn't be in your patch. Make sure you are posting correctly with rbt post -u branch1..branch2

brenniebrennie

Undo this.

brenniebrennie

Remove this.

brenniebrennie

Can you format this as: status = ('%sOpen Issues (%s)%s' % (Fore.YELLOW, request.issue_open_count, Fore.RESET)) with appropriate wrapping?

brenniebrennie

Likewise here.

brenniebrennie

'ColoredFormatter' imported but unused

reviewbotreviewbot

This should be renamed to status_width and the result is no longer always len(status). Becuase status contains control codes, which …

brenniebrennie
reviewbot
  1. 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
    
    
  2. rbtools/commands/status.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (84 > 79 characters)
    
  3. 
      
DA
JW
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 1)
     
     

    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.

    1. 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
      
  3. 
      
brennie
  1. 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.

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

    Needs a space at end of line.

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

    Needs a ,.

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

    Needs a ,.

  5. setup.py (Diff revision 1)
     
     

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

  6. 
      
brennie
  1. 
      
  2. rbtools/commands/status.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     

    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

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

    See other comment.

  4. 
      
DA
reviewbot
  1. 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
    
    
  2. rbtools/commands/status.py (Diff revision 2)
     
     
    Col: 33
     E127 continuation line over-indented for visual indent
    
  3. rbtools/commands/status.py (Diff revision 2)
     
     
    Col: 33
     E127 continuation line over-indented for visual indent
    
  4. 
      
DA
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. rbtools/commands/status.py (Diff revision 3)
     
     

    Don't need this here.

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

    Remove this.

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

    You need to do add Fore.RESET to the end of these strings.

  5. setup.py (Diff revision 3)
     
     
     
     
     
     

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

  6. 
      
DA
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 4)
     
     
     

    Undo this change.

  3. setup.py (Diff revision 4)
     
     

    Undo this

  4. 
      
DA
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. Let's touch base and we can go over fixing up your git issues.

  2. rbtools/commands/__init__.py (Diff revision 5)
     
     

    This shouldn't be in this patch.

  3. setup.py (Diff revision 5)
     
     
     
     
     
     

    This shouldn't be in your patch.

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

  4. setup.py (Diff revision 5)
     
     
     

    Undo this.

  5. 
      
DA
reviewbot
  1. 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
    
    
  2. 
      
DA
reviewbot
  1. 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
    
    
  2. 
      
brennie
  1. 
      
  2. rbtools/commands/status.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     

    Remove this.

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

    Can you format this as:

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

    with appropriate wrapping?

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

    Likewise here.

  5. 
      
DA
reviewbot
  1. 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
    
    
  2. rbtools/commands/status.py (Diff revision 8)
     
     
     'ColoredFormatter' imported but unused
    
  3. 
      
DA
reviewbot
  1. 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
    
    
  2. 
      
brennie
DA
brennie
  1. 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.

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

    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.

  3. 
      
DA
Review request changed

Status: Discarded

Loading...