Summary

Add pydocstyle tool to Review Bot

Review Request #10197 — Created Oct. 4, 2018 and submitted Nov. 26, 2018, 3:56 p.m.

Information

Owner

ilaw

Repository

ReviewBot

Branch

master

Bugs

Depends On

Commit

738b160...

Reviewers

Groups

reviewbot, students

People

Description

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.

Testing Done

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.

Issues

Description	From	Last Updated
In your summary and description, "ReviewBot" -> "Review Bot"	david	Nov. 1, 2018, 4:13 p.m.
We also need to add documentation for the pydocstyle tool.	david	Nov. 1, 2018, 4:13 p.m.
E501 line too long (88 > 79 characters)	reviewbot	Oct. 10, 2018, 4:51 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Oct. 10, 2018, 4:51 p.m.
E501 line too long (85 > 79 characters)	reviewbot	Oct. 10, 2018, 4:51 p.m.
E226 missing whitespace around arithmetic operator	reviewbot	Oct. 10, 2018, 4:51 p.m.
F401 'reviewbot.processing.review.File' imported but unused	reviewbot	Oct. 10, 2018, 4:51 p.m.
F401 'reviewbot.processing.review.Review' imported but unused	reviewbot	Oct. 10, 2018, 4:51 p.m.
F401 'rbtools.api.resource.Resource' imported but unused	reviewbot	Oct. 10, 2018, 4:51 p.m.
F401 'pydocstyle' imported but unused	reviewbot	Oct. 10, 2018, 4:51 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 10, 2018, 4:51 p.m.
W391 blank line at end of file	reviewbot	Oct. 10, 2018, 4:51 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 10, 2018, 4:51 p.m.
Can you add a module-level docstring describing what the file is/does?	brennie	Oct. 17, 2018, 11:38 p.m.
"PEP 257"	brennie	Oct. 17, 2018, 11:38 p.m.
This patch should build upon your previous patch and add pydocstyle to the all extra_requires section.	brennie	Oct. 17, 2018, 11:38 p.m.
Alphabetical order.	brennie	Oct. 23, 2018, 11:27 p.m.
Please use single quotes instead of double.	david	Oct. 28, 2018, 5:58 p.m.
Please use single quotes instead of double here.	david	Oct. 28, 2018, 5:58 p.m.
This is rst, which has a somewhat different markup for code formatting than markdown. This should probably be: :command:`pydocstyle` can …	david	Nov. 1, 2018, 5:06 p.m.
I don't believe these lines are needed here, only the (duplicated) docstring under your class PydocstyleTool on line 9.	gojeffcho	Nov. 10, 2018, 12:42 p.m.
Should this be looking for something a little more specific than the broad Exception class?	gojeffcho	Nov. 10, 2018, 12:49 p.m.

flake8 failed.

JSHint passed.

flake8

bot/reviewbot/tools/pydocstyle.py (Diff revision 1)
Show all issues
```
E501 line too long (88 > 79 characters)
```
bot/reviewbot/tools/pydocstyle.py (Diff revision 1)
Show all issues
```
E501 line too long (84 > 79 characters)
```
bot/reviewbot/tools/pydocstyle.py (Diff revision 1)
Show all issues
```
E501 line too long (85 > 79 characters)
```
bot/reviewbot/tools/pydocstyle.py (Diff revision 1)
Show all issues
```
E226 missing whitespace around arithmetic operator
```

bot/reviewbot/tools/tests.py (Diff revision 1)

Show all issues

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

bot/reviewbot/tools/tests.py (Diff revision 1)

Show all issues

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

bot/reviewbot/tools/tests.py (Diff revision 1)

Show all issues

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

bot/reviewbot/tools/tests.py (Diff revision 1)
Show all issues
```
F401 'pydocstyle' imported but unused
```
bot/reviewbot/tools/tests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
bot/reviewbot/tools/tests.py (Diff revision 1)
Show all issues
```
W391 blank line at end of file
```
bot/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```

Summary:

-	[WIP] Adding pydocstyle tool to ReviewBot and testing
+	Add pydocstyle tool to ReviewBot

Description:

~		[WIP] Adding pydocstyle tool to ReviewBot and testing
	~	Users can now configure ReviewBot to run pydocstyle, a tool for checking
	+	compliance with Python docstring conventions.

~		Added pydocstyle tool to setup and implementation
~		Tested tool manually
~		Created runtests.py for ReviewBot (which is similar to the other packages)
~		Working on creating unit tests for pydocstyletool
	~	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.

Testing Done:

	+	Tested dependency check by running reviewbot worker command without
	+	pydocstyle installed
	+
	+	Tested pydocstyle by adding new configuration to ReviewBot integration
	+	and publishing a review. Also tested with errors added to ignore list.

Commit:

-	85e05234119d6d003668e1930f2c59cdc6e0d74a
+	422c783bd814e8c2185bac26595a559f25182580

Diff:

Revision 2 (+78)

Show changes

	bot/setup.py
	bot/reviewbot/tools/pydocstyle.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

bot/reviewbot/tools/pydocstyle.py (Diff revision 2)

Show all issues

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

ilaw Oct. 11, 2018, 8:09 p.m.

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

bot/reviewbot/tools/pydocstyle.py (Diff revision 2)
Show all issues
```
"PEP 257"
```

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.

Commit:

-	422c783bd814e8c2185bac26595a559f25182580
+	342715177b87d03f7e2d8eb89ac5af06801d1e56

Diff:

Revision 3 (+81)

Show changes

	bot/setup.py
	bot/reviewbot/tools/pydocstyle.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

bot/setup.py (Diff revision 3)
Show all issues
```
Alphabetical order.
```

Commit:

-	342715177b87d03f7e2d8eb89ac5af06801d1e56
+	0761b0addc23a176e38b516d82e111afb603bcca

Diff:

Revision 4 (+81)

Show changes

	bot/setup.py
	bot/reviewbot/tools/pydocstyle.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

bot/reviewbot/tools/pydocstyle.py (Diff revision 4)
Show all issues
```
Please use single quotes instead of double.
```
bot/reviewbot/tools/pydocstyle.py (Diff revision 4)
Show all issues
```
Please use single quotes instead of double here.
```

Commit:

-	0761b0addc23a176e38b516d82e111afb603bcca
+	a89490291324d1e4650eceeb8676140a769ab029

Diff:

Revision 5 (+81)

Show changes

	bot/setup.py
	bot/reviewbot/tools/pydocstyle.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Show all issues

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

Show all issues

We also need to add documentation for the pydocstyle tool.

Summary:

-	Add pydocstyle tool to ReviewBot
+	Add pydocstyle tool to Review Bot

Description:

~		Users can now configure ReviewBot to run pydocstyle, a tool for checking
	~	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.

Testing Done:

		Tested dependency check by running reviewbot worker command without
		pydocstyle installed

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

Commit:

-	a89490291324d1e4650eceeb8676140a769ab029
+	01535fc6a0d209e02dc0df75d64e6ba7dc72aa83

Diff:

Revision 6 (+111)

Show changes

	README.rst
	bot/README.rst
	bot/setup.py
	bot/reviewbot/tools/pydocstyle.py
	docs/reviewbot/tools/index.rst
	docs/reviewbot/tools/pydocstyle.rst
	extension/README.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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.

david Nov. 1, 2018, 4:21 p.m.

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.

Commit:

-	01535fc6a0d209e02dc0df75d64e6ba7dc72aa83
+	738b16075bf5157c06e46cbda2b3e094091cec42

Diff:

Revision 7 (+114)

Show changes

	README.rst
	bot/README.rst
	bot/setup.py
	bot/reviewbot/tools/pydocstyle.py
	docs/reviewbot/tools/index.rst
	docs/reviewbot/tools/pydocstyle.rst
	extension/README.rst

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

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.

ilaw Nov. 8, 2018, 11:11 a.m.

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

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?

ilaw Nov. 10, 2018, 12:48 p.m.

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

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (6773084)