• 
      

    Dashboard bugs column as clickable links to bug tracker system (if defined)

    Review Request #3275 — Created Aug. 13, 2012 and discarded

    Information

    Review Board
    main

    Reviewers

    Makes the bug numbers into clickable links if a bug tracker system is associated with the review.
    Tested on local environment with and without a bug tracker template.
    Description From Last Updated

    Imports must be alphabetized.

    chipx86 chipx86

    Two blank lines between classes.

    chipx86 chipx86

    I know some older classes don't do this right, but this should be: super(Column, self).__init__(_('Bugs'), ...)

    chipx86 chipx86

    No space before ':'

    chipx86 chipx86

    You can actually simplify this with the very neat Python generators: return ','.join([ '%s' % (bug_url % bug, url) for …

    chipx86 chipx86

    This should include the format, and ideally say "... when rendering bugs column". So: logging.warning('Invalid bug tracker format when rendering …

    chipx86 chipx86

    Two blank lines.

    chipx86 chipx86

    Alignment issues. Make sure you're not using tabs in any code. It should always be 4 space indentation.

    chipx86 chipx86

    No need for the \, since it's in parens.

    chipx86 chipx86

    We can just unconditionally return this at the end. No need to do it once in the exception handler and …

    chipx86 chipx86

    Can you make the = align with the other columns?

    chipx86 chipx86

    Alignment issue.

    chipx86 chipx86

    Probably should be ", ", actually. A space will make it look nicer, and will allow it to wrap if …

    chipx86 chipx86

    Blank line between these.

    chipx86 chipx86
    chipx86
    1. This looks like a good addition! There's some style things to fix up, and there's a nice trick you can use, but the logic looks generally fine.
    2. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
       
      Show all issues
      Imports must be alphabetized.
    3. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
       
       
      Show all issues
      Two blank lines between classes.
    4. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
      Show all issues
      I know some older classes don't do this right, but this should be:
      
      super(Column, self).__init__(_('Bugs'), ...)
      1. I presume this should be: super(BugsColumn, self).__init__(_('Bugs'), ...) ?
      2. Oops, yes.
    5. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
      Show all issues
      No space before ':'
    6. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
       
       
       
       
      Show all issues
      You can actually simplify this with the very neat Python generators:
      
      return ','.join([
          '<a href="%s">%s</a>' % (bug_url % bug, url)
          for bug in bugs
      ])
    7. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
      Show all issues
      This should include the format, and ideally say "... when rendering bugs column". So:
      
      logging.warning('Invalid bug tracker format when rendering bugs column: %s' % bug_url)
    8. reviewboard/reviews/datagrids.py (Diff revision 1)
       
       
       
       
      Show all issues
      Two blank lines.
    9. 
        
    TC
    chipx86
    1. 
        
    2. reviewboard/reviews/datagrids.py (Diff revision 2)
       
       
       
       
      Show all issues
      Alignment issues.
      
      Make sure you're not using tabs in any code. It should always be 4 space indentation.
      1. I really ought to review PEP 8. I found a cool vim plugin that will help me out in the future at least.
    3. reviewboard/reviews/datagrids.py (Diff revision 2)
       
       
      Show all issues
      No need for the \, since it's in parens.
      
      
    4. reviewboard/reviews/datagrids.py (Diff revision 2)
       
       
       
       
      Show all issues
      We can just unconditionally return this at the end. No need to do it once in the exception handler and once in the else statement.
    5. reviewboard/reviews/datagrids.py (Diff revision 2)
       
       
      Show all issues
      Can you make the = align with the other columns?
    6. 
        
    TC
    chipx86
    1. This looks good in general, but one thing I'd like to see is the links become blue and underlined in the dashboard, so it's clear they're clickable.
      
      You can have a class for the column and use that to key off the CSS.
    2. reviewboard/reviews/datagrids.py (Diff revision 3)
       
       
       
      Show all issues
      Alignment issue.
    3. reviewboard/reviews/datagrids.py (Diff revision 3)
       
       
      Show all issues
      Probably should be ", ", actually. A space will make it look nicer, and will allow it to wrap if there's a large number of bugs.
    4. reviewboard/reviews/datagrids.py (Diff revision 3)
       
       
       
      Show all issues
      Blank line between these.
    5. 
        
    TC
    Review request changed
    Status:
    Discarded