Add pydocstyle tool to Review Bot

Review Request #10197 — Created Oct. 4, 2018 and submitted

Information

ReviewBot
master
738b160...

Reviewers

Users can now configure Review Bot to run pydocstyle, a tool for checking
compliance with Python docstring conventions.

Pydocstyle is not added to the install requires list and users will
need to manually install it to use it.

Pydocstyle defaults to using the PEP 257 docstring convention but users
can add a list of errors to ignore which will override the convention.

Tested dependency check by running reviewbot worker command without
pydocstyle installed

Tested pydocstyle by adding new configuration to Review Bot integration
and publishing a review. Also tested with errors added to ignore list.

Description From Last Updated

In your summary and description, "ReviewBot" -> "Review Bot"

daviddavid

We also need to add documentation for the pydocstyle tool.

daviddavid

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E226 missing whitespace around arithmetic operator

reviewbotreviewbot

F401 'reviewbot.processing.review.File' imported but unused

reviewbotreviewbot

F401 'reviewbot.processing.review.Review' imported but unused

reviewbotreviewbot

F401 'rbtools.api.resource.Resource' imported but unused

reviewbotreviewbot

F401 'pydocstyle' imported but unused

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

W391 blank line at end of file

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

Can you add a module-level docstring describing what the file is/does?

brenniebrennie

"PEP 257"

brenniebrennie

This patch should build upon your previous patch and add pydocstyle to the all extra_requires section.

brenniebrennie

Alphabetical order.

brenniebrennie

Please use single quotes instead of double.

daviddavid

Please use single quotes instead of double here.

daviddavid

This is rst, which has a somewhat different markup for code formatting than markdown. This should probably be: :command:`pydocstyle` can …

daviddavid

I don't believe these lines are needed here, only the (duplicated) docstring under your class PydocstyleTool on line 9.

gojeffchogojeffcho

Should this be looking for something a little more specific than the broad Exception class?

gojeffchogojeffcho
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

ilaw
brennie
  1. 
      
  2. bot/reviewbot/tools/pydocstyle.py (Diff revision 2)
     
     
    Show all issues

    Can you add a module-level docstring describing what the file is/does?

    1. Should I make it the same as the class-level docstring ("""Review Bot tool to run pydocstyle.""") which is what flake8.py does?

  3. bot/reviewbot/tools/pydocstyle.py (Diff revision 2)
     
     
    Show all issues

    "PEP 257"

  4. 
      
brennie
  1. 
      
  2. bot/setup.py (Diff revision 2)
     
     
    Show all issues

    This patch should build upon your previous patch and add pydocstyle to the all extra_requires section.

  3. 
      
ilaw
brennie
  1. 
      
  2. bot/setup.py (Diff revision 3)
     
     
     
     
    Show all issues

    Alphabetical order.

  3. 
      
ilaw
david
  1. 
      
  2. bot/reviewbot/tools/pydocstyle.py (Diff revision 4)
     
     
    Show all issues

    Please use single quotes instead of double.

  3. bot/reviewbot/tools/pydocstyle.py (Diff revision 4)
     
     
    Show all issues

    Please use single quotes instead of double here.

  4. 
      
ilaw
david
  1. 
      
  2. Show all issues

    In your summary and description, "ReviewBot" -> "Review Bot"

  3. Show all issues

    We also need to add documentation for the pydocstyle tool.

  4. 
      
ilaw
david
  1. 
      
  2. docs/reviewbot/tools/pydocstyle.rst (Diff revision 6)
     
     
     
    Show all issues

    This is rst, which has a somewhat different markup for code formatting than markdown. This should probably be:

    :command:`pydocstyle` can be installed on most systems by running::
    
        $ pip install pydocstyle
    

    I notice now that the earlier changes made this same mistake, so I'll fix up the ones that are already in the tree.

    1. Here's the full text I'm using:

      :command:`pyflakes` can be installed on most systems by running::
      
          $ pip install pyflakes
      
      It may also be available in your system's package manager.
      
  3. 
      
ilaw
david
  1. Ship It!
  2. 
      
gojeffcho
  1. 
      
  2. bot/reviewbot/tools/pydocstyle.py (Diff revision 7)
     
     
     
    Show all issues

    I don't believe these lines are needed here, only the (duplicated) docstring under your class PydocstyleTool on line 9.

    1. Barret said to add a module-level docstring in addition to the class-level docstring

  3. bot/reviewbot/tools/pydocstyle.py (Diff revision 7)
     
     
    Show all issues

    Should this be looking for something a little more specific than the broad Exception class?

    1. I think it's good to have it as a catch-all in case there were parsing issues and if some lines don't have the format we want

  4. 
      
david
  1. Ship It!
  2. 
      
ilaw
Review request changed
Status:
Completed
Change Summary:
Pushed to master (6773084)