Summary

Fix PEP-257 violations in reviewboard/admin/

Review Request #7060 — Created March 13, 2015 and submitted March 17, 2015, 2:49 p.m.

Information

Owner

david

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

25d3674...

Reviewers

Groups

reviewboard

People

Description

This fixes up existing docstrings to conform to PEP-8 and PEP-257 guidelines,

and adds all missing class and method docstrings in the reviewboard/admin/

directory.

Testing Done

Ran pep257 --ignore=D100,D203 (disables module docstrings and the blank line

before class docstring rules). The only remaining errors were with the way we

wrap our long testcase docstrings.

Issues

Description	From	Last Updated
Should remove the "the".	chipx86	March 16, 2015, 1:01 p.m.
We should say what the validation functions are checking for, in all the clean functions.	chipx86	March 16, 2015, 1:03 p.m.
'django_reset' imported but unused	reviewbot	March 13, 2015, 4:51 p.m.
ModPythonRequest should just go away.	chipx86	March 16, 2015, 1:05 p.m.
Looking at the source, I think we always have WSGIRequest these days. We can probably just remove this try/except.	chipx86	March 16, 2015, 1:05 p.m.
This should say what it does.	chipx86	March 16, 2015, 1:07 p.m.
Same.	chipx86	March 16, 2015, 1:07 p.m.
Missing a closing paren.	chipx86	March 16, 2015, 1:07 p.m.
And here.	chipx86	March 16, 2015, 1:07 p.m.
This should mention it must be subclassed or will raise an exception.	chipx86	March 16, 2015, 1:08 p.m.
This should say what it's doing.	chipx86	March 16, 2015, 1:09 p.m.
And here.	chipx86	March 16, 2015, 1:09 p.m.
And here.	chipx86	March 16, 2015, 1:09 p.m.
"identical"	chipx86	March 16, 2015, 1:09 p.m.
This should say what it's doing.	chipx86	March 16, 2015, 1:10 p.m.
"site configuration"	chipx86	March 16, 2015, 1:10 p.m.
Not sure ones like these are worth it. It's not a publicly-exposed class in any way, and the docstring is …	chipx86	March 16, 2015, 1:10 p.m.
Should say "URL," since we're explicitly dealing with those.	chipx86	March 16, 2015, 1:11 p.m.
Missing paren.	chipx86	March 16, 2015, 1:11 p.m.
"update"	chipx86	March 16, 2015, 1:11 p.m.
'django_reset' imported but unused	reviewbot	March 16, 2015, 1:16 p.m.
X509 should be capitalized.	brennie	March 17, 2015, 2:48 p.m.
""" on next line	brennie	March 17, 2015, 2:49 p.m.
Should this be "an <li>" because of the way <li> is pronounced ("ell eye") ?	brennie	March 17, 2015, 2:49 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/admin/checks.py
    reviewboard/admin/forms.py
    reviewboard/admin/management/commands/dumpdb.py
    reviewboard/admin/support.py
    reviewboard/admin/management/evolutions.py
    reviewboard/admin/views.py
    reviewboard/admin/siteconfig.py
    reviewboard/admin/templatetags/rbadmintags.py
    reviewboard/admin/server.py
    reviewboard/admin/management/commands/resolve-check.py
    reviewboard/admin/management/commands/loaddb.py
    reviewboard/admin/validation.py
    reviewboard/admin/management/sites.py
    reviewboard/admin/middleware.py
    reviewboard/admin/widgets.py
    reviewboard/admin/context_processors.py
    reviewboard/admin/security_checks.py
    reviewboard/admin/cache_stats.py
    reviewboard/admin/import_utils.py
    reviewboard/admin/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/checks.py
    reviewboard/admin/forms.py
    reviewboard/admin/management/commands/dumpdb.py
    reviewboard/admin/support.py
    reviewboard/admin/management/evolutions.py
    reviewboard/admin/views.py
    reviewboard/admin/siteconfig.py
    reviewboard/admin/templatetags/rbadmintags.py
    reviewboard/admin/server.py
    reviewboard/admin/management/commands/resolve-check.py
    reviewboard/admin/management/commands/loaddb.py
    reviewboard/admin/validation.py
    reviewboard/admin/management/sites.py
    reviewboard/admin/middleware.py
    reviewboard/admin/widgets.py
    reviewboard/admin/context_processors.py
    reviewboard/admin/security_checks.py
    reviewboard/admin/cache_stats.py
    reviewboard/admin/import_utils.py
    reviewboard/admin/tests.py

reviewboard/admin/management/commands/loaddb.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```

reviewboard/admin/context_processors.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should remove the "the".
```

reviewboard/admin/forms.py (Diff revision 1)

The issue has been resolved. Show all issues

We should say what the validation functions are checking for, in all the clean functions.

reviewboard/admin/forms.py (Diff revision 1)

Docstrings are good, but I feel like simple ones like this that basically duplicate the function name (same with some of the "save" functions) aren't really worth it. I mean, it doesn't hurt, and I wouldn't say to remove any of them, but.. well.. thoughts?

david March 16, 2015, 1:11 p.m.

I want to get to the point where we can add pep257 to Review Bot. These docstrings are silly, but I don't think it hurts.

reviewboard/admin/middleware.py (Diff revision 1)

The issue has been dropped. Show all issues

ModPythonRequest should just go away.

david March 16, 2015, 1:11 p.m.

For better or worse, we do still have people with mod_python based deployments, and I'd rather not break their authentication.

reviewboard/admin/middleware.py (Diff revision 1)

The issue has been resolved. Show all issues

Looking at the source, I think we always have WSGIRequest these days. We can probably just remove this try/except.

reviewboard/admin/middleware.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should say what it does.
```
reviewboard/admin/middleware.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same.
```
reviewboard/admin/middleware.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing a closing paren.
```
reviewboard/admin/middleware.py (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```
reviewboard/admin/security_checks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should mention it must be subclassed or will raise an exception.
```
reviewboard/admin/security_checks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should say what it's doing.
```
reviewboard/admin/security_checks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```
reviewboard/admin/security_checks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
And here.
```
reviewboard/admin/security_checks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"identical"
```
reviewboard/admin/security_checks.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should say what it's doing.
```
reviewboard/admin/support.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"site configuration"
```

reviewboard/admin/tests.py (Diff revision 1)

The issue has been dropped. Show all issues

Not sure ones like these are worth it. It's not a publicly-exposed class in any way, and the docstring is really just extra redundant noise.

reviewboard/admin/validation.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should say "URL," since we're explicitly dealing with those.
```
reviewboard/admin/validation.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing paren.
```
reviewboard/admin/views.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"update"
```

Commit:

-	ee55c84512abb3467a874f82372e1b461eefd338
+	25d36744a43ad6254ae8d813a16a9afef19956d5

Diff:

Revision 2 (+257 -111)

Show changes

	reviewboard/admin/cache_stats.py
	reviewboard/admin/checks.py
	reviewboard/admin/context_processors.py
	reviewboard/admin/forms.py
	reviewboard/admin/import_utils.py
	reviewboard/admin/middleware.py
	reviewboard/admin/security_checks.py
	reviewboard/admin/server.py
	12 more

Tool: Pyflakes
Processed Files:
    reviewboard/admin/checks.py
    reviewboard/admin/forms.py
    reviewboard/admin/management/commands/dumpdb.py
    reviewboard/admin/support.py
    reviewboard/admin/management/evolutions.py
    reviewboard/admin/views.py
    reviewboard/admin/siteconfig.py
    reviewboard/admin/templatetags/rbadmintags.py
    reviewboard/admin/server.py
    reviewboard/admin/management/commands/resolve-check.py
    reviewboard/admin/management/commands/loaddb.py
    reviewboard/admin/validation.py
    reviewboard/admin/management/sites.py
    reviewboard/admin/middleware.py
    reviewboard/admin/widgets.py
    reviewboard/admin/context_processors.py
    reviewboard/admin/security_checks.py
    reviewboard/admin/cache_stats.py
    reviewboard/admin/import_utils.py
    reviewboard/admin/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/checks.py
    reviewboard/admin/forms.py
    reviewboard/admin/management/commands/dumpdb.py
    reviewboard/admin/support.py
    reviewboard/admin/management/evolutions.py
    reviewboard/admin/views.py
    reviewboard/admin/siteconfig.py
    reviewboard/admin/templatetags/rbadmintags.py
    reviewboard/admin/server.py
    reviewboard/admin/management/commands/resolve-check.py
    reviewboard/admin/management/commands/loaddb.py
    reviewboard/admin/validation.py
    reviewboard/admin/management/sites.py
    reviewboard/admin/middleware.py
    reviewboard/admin/widgets.py
    reviewboard/admin/context_processors.py
    reviewboard/admin/security_checks.py
    reviewboard/admin/cache_stats.py
    reviewboard/admin/import_utils.py
    reviewboard/admin/tests.py

reviewboard/admin/management/commands/loaddb.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```

Ship it!

reviewboard/admin/middleware.py (Diff revision 2)
The issue has been resolved. Show all issues
```
X509 should be capitalized.
```
reviewboard/admin/security_checks.py (Diff revision 2)
The issue has been resolved. Show all issues
```
""" on next line
```
reviewboard/admin/templatetags/rbadmintags.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Should this be "an <li>" because of the way <li> is pronounced ("ell eye") ?
```

Status: Closed (submitted)

Change Summary:

Pushed to master (7c4d12a)