[WIP]ReviewBot PEP257Tool

Review Request #7101 - Created March 22, 2015 and discarded

Jifei Wen
ReviewBot
release-0.2.x
reviewboard, reviewbot, students
brennie

pep257 tool

pip3 install pep257
or
easy_install pep257

  • 7
  • 0
  • 83
  • 1
  • 91
Description From Last Updated
You can format this so it takes fewer lines, e.g. output = execute(['pep257', path], split_lines=True, ignore_errors=True) Barret Rennie Barret Rennie
This isn't really a model. Barret Rennie Barret Rennie
Blank line between paragraphs in docstrings. Barret Rennie Barret Rennie
"Review Bot" "interface to" Barret Rennie Barret Rennie
This should go before the _MSG_START constant. Also, make sure to use proper capitalization and ensure that it ends with ... Barret Rennie Barret Rennie
Blank line between statement and block. Barret Rennie Barret Rennie
You should maybe comment on the format of the output of pep257. You can just put an example of the ... Barret Rennie Barret Rennie
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/pep257.py
        bot/reviewbot/tools/pep8.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/pep257.py
        bot/reviewbot/tools/pep8.py
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 1)
     
     
    Col: 20
     E262 inline comment should start with '# '
    
  3. bot/reviewbot/tools/pep257.py (Diff revision 1)
     
     
     undefined name 'lnum'
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 1)
     
     
     local variable 'lnum' is assigned to but never used
    
  5. bot/reviewbot/tools/pep257.py (Diff revision 1)
     
     
    Col: 20
     W292 no newline at end of file
    
  6. bot/reviewbot/tools/pep8.py (Diff revision 1)
     
     
    Col: 12
     E225 missing whitespace around operator
    
  7. bot/reviewbot/tools/pep8.py (Diff revision 1)
     
     
    Col: 16
     E225 missing whitespace around operator
    
  8. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/pep257.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/pep257.py
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  3. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     undefined name 'lnum'
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     undefined name 'level'
    
  5. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     undefined name 'level'
    
  6. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     undefined name 'lnum'
    
  7. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     local variable 'lnum' is assigned to but never used
    
  8. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     local variable 'level' is assigned to but never used
    
  9. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
    Col: 20
     W292 no newline at end of file
    
  10. 
      
Barret Rennie
  1. 
      
  2. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     

    Blank line between these.

    This class also needs a docstring.

  3. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     
     

    These should all have values, I believe.

  4. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    Needs docstring.

  5. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    Needs docstring.

  6. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    This comment is unnecessary; the code is self explanatory.

  7. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     
     
     
     
     
     

    You can format this so it takes fewer lines, e.g.

            output = execute(['pep257', path],
                             split_lines=True,
                             ignore_errors=True)
    
    1. Please do not mark issues as fixed when they are not fixed.

    2. this is the previous code. i have change it like:

      output = execute(
      [
      'pep257',
      '--ignore=%s' % self.settings['ignore'],
      path
      ],
      split_lines=True,
      ignore_errors=True)

    3. The comments re: formatting still apply. It should then be formatted like so:

              output = execute(['pep257', '--ignore=%s' % self.settings['ignore'],
                                path],
                               split_lines=True,
                               ignore_errors=True)
      
  8. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    What happens if execution fails? You should maybe specify none_on_ignored_error in the execute call and check if it returns None.

    1. if the out put is None. the for loop will not execute. and it will only return True. No comment will post like the pep257 no result if it have no error.
      and i wonder what you are asking me to do.

    2. Actually, it appears it will raise an exception:

      >>> for _ in None:
      ...     print('In the loop!')
      ...
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      TypeError: 'NoneType' object is not iterable
      
  9. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     

    This needs some work.

    I assume that a line will not start with the special ' ' string the first time and so lnum and level will get set. Is this guaranteed? If not, this needs to be more robust.

    However, the static analysis tool doesn't know that. You should set lnum and level to None before the for loop.

  10. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    This is a weird string and we should document it.

    1. Sorry, I don't you what you mean.
      The PEP257's result is like:
      pep257.py:1 at module level:
      D100: Missing docstring in public module
      I should use it to differ the error code line and the number line.

    2. I just mean that we should document that this is the start of a line that looks like that.

      We should probably just document the format of PEP257's output.

  11. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     
     
     

    Left over from debugging?

  12. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    Is this the line number? if so, can we rename this to line_number ?

    Also, this line number needs to be checked to see that is within the changed lines in the diff. We don't want to comment on unchanged lines.

    1. get it.
      the pep8.py is wtitten like it.

  13. bot/reviewbot/tools/pep257.py (Diff revision 2)
     
     

    What is level?

    1. the result is like:
      pep257.py:1 at module level:
      D100: Missing docstring in public module
      i do not konw how to descript at module level

      and i can not find the entry point.
      what file is the entry point in.

  14. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/pep257.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/pep257.py
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 13
     E126 continuation line over-indented for hanging indent
    
  3. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  5. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  6. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  7. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 9
     E122 continuation line missing indentation or outdented
    
  8. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 10
     E231 missing whitespace after ','
    
  9. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  10. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
     undefined name 'lnum'
    
  11. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
     undefined name 'level'
    
  12. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
     local variable 'lnum' is assigned to but never used
    
  13. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
     local variable 'level' is assigned to but never used
    
  14. bot/reviewbot/tools/pep257.py (Diff revision 3)
     
     
    Col: 20
     W292 no newline at end of file
    
  15. 
      
Jifei Wen
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/pep257.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/pep257.py
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 13
     E126 continuation line over-indented for hanging indent
    
  3. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  5. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  6. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 13
     E122 continuation line missing indentation or outdented
    
  7. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 9
     E122 continuation line missing indentation or outdented
    
  8. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 10
     E231 missing whitespace after ','
    
  9. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  10. bot/reviewbot/tools/pep257.py (Diff revision 4)
     
     
    Col: 20
     W292 no newline at end of file
    
  11. 
      
Jifei Wen
Jifei Wen
Jifei Wen
Jifei Wen
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 7)
     
     
    Col: 80
     E501 line too long (112 > 79 characters)
    
  3. 
      
Jifei Wen
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
    Col: 80
     E501 line too long (112 > 79 characters)
    
  3. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
    Col: 80
     E501 line too long (103 > 79 characters)
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  5. 
      
Barret Rennie
  1. Can you add the students review group?

  2. 
      
Barret Rennie
  1. 
      
  2. bot/reviewbot-cookies.txt (Diff revision 8)
     
     
     
     
     
     

    This file should not be in the diff.

    1. get it

    2. The file is still in your review request. Please do not mark issues as fixed when they are not fixed.

    3. i have git rm it from my local repository.how can i delete it from reviewboard?

    4. After you've done git rm you need to make sure to commit your changes.

      If that for some reason does not work, you can ask in Slack or run rbt post -X/bot/reviewbot-cookies.txt -r7101 and the file will be excluded from the post.

    5. Ah, I've see you've already fixed this so you can ignore that comment.

  3. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
     
     
     
     
     
     
     
     
     

    Can this be formatted like:

    options = [
        {
            'name': 'ignore',
            # ...
            'field_options': {
                'label': 'Ignore',
                # ...
            },
        },
    ]
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
     
     
     
     

    Docstrings should be of the form

    """Single line summary.

    Multi-line description.
    """

  5. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
     
     
     
     
     

    Docstrings should be of the form

    """Single line summary.

    Multi-line description.
    """

  6. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     

    We don't really use :param foo: in the codebase.

  7. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
     

    Blank line between these.

  8. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
     

    Blank line between these.

  9. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Since you're returning True in either case, you can do

    ```python
    if output:
    for line in output:
    # ...

    return True
    ```

  10. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     

    Again, can you make ' ' * 8 a constant on the class? You can document it there.

    1. sorry, i did not know what's your meaning last time.

    2. Can you make a variable on the class like _MESSAGE_START = ' ' * 8.

      By on the class I mean under the class statement and not in a function definition.

  11. bot/reviewbot/tools/pep257.py (Diff revision 8)
     
     

    Is the code helpful?

    1. you mean the pep257's warning code?
      i think a coder can search the code in the document of pep257, then he can know what should he modify his docstring.

  12. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 9)
     
     
    Col: 29
     E131 continuation line unaligned for hanging indent
    
  3. bot/reviewbot/tools/pep257.py (Diff revision 9)
     
     
    Col: 80
     E501 line too long (104 > 79 characters)
    
  4. bot/reviewbot/tools/pep257.py (Diff revision 9)
     
     
    Col: 80
     E501 line too long (86 > 79 characters)
    
  5. bot/reviewbot/tools/pep257.py (Diff revision 9)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  6. 
      
Jifei Wen
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 10)
     
     
    Col: 29
     E131 continuation line unaligned for hanging indent
    
  3. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 12)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 12)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  4. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/reviewbot/tools/shellcheck.py
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. bot/reviewbot/tools/shellcheck.py (Diff revision 13)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  4. 
      
Jifei Wen
Review Bot
  1. Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
    
    Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        bot/reviewbot-cookies.txt
    
    
  2. 
      
Jifei Wen
Barret Rennie
  1. 
      
  2. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     

    This needs a better docstring.

  3. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     

    Single quotes for strings.

  4. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     
     

    Docstrings should be of the format:

    """Single line summary.

    Multi-line description.
    """

  5. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     
     
     

    Docstrings should be of the format:

    """Single line summary.

    Multi-line description.
    """

  6. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     

    Blank line between block and statement.

  7. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     

    Blank line between statement and block.

  8. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     

    Blank line between block and statement.

  9. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     

    This should be on the class (with the comment before it). See issue in my previous review.

  10. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     

    Blank line between these.

  11. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     

    I'd prefer len(msg_start) over just 8 here.

  12. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     
     
     

    Why not

    code, msg = line[len(msg_start):].split(':', 1)
    
  13. bot/reviewbot/tools/pep257.py (Diff revision 14)
     
     
     

    Blank line between end of block and a statement.

  14. 
      
Jifei Wen
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        .gitignore
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        .gitignore
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 15)
     
     
    Col: 5
     E301 expected 1 blank line, found 0
    
  3. 
      
Jifei Wen
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        .gitignore
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        bot/setup.py
        bot/reviewbot/tools/pep257.py
    
    Ignored Files:
        .gitignore
    
    
  2. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     
    Col: 5
     E301 expected 1 blank line, found 0
    
  3. 
      
Barret Rennie
  1. <p>It seems you've accidentally removed the <code>.gitignore</code> from the repository. Please undo this change.</p>
    <p>You can undo this by doing <code>git checkout master -- .gitignore</code> and then commiting the change.</p>

  2. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     

    This isn't really a model.

  3. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     
     

    Blank line between paragraphs in docstrings.

  4. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     

    "Review Bot"

    "interface to"

  5. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     

    This should go before the _MSG_START constant.

    Also, make sure to use proper capitalization and ensure that it ends with a period.

  6. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     
     

    Blank line between statement and block.

  7. bot/reviewbot/tools/pep257.py (Diff revision 16)
     
     
     

    You should maybe comment on the format of the output of pep257. You can just put an example of the output, that would be fine.

  8. 
      
Jifei Wen
Review request changed

Status: Discarded

Loading...