Summary

Do some clean-up on the ReviewBot worker code.

Review Request #8433 — Created Sept. 24, 2016 and submitted Oct. 26, 2016, 1:55 p.m.

Information

Owner

david

Repository

ReviewBot

Branch

master

Bugs

Depends On

Commit

e13d365...

Reviewers

Groups

reviewbot

People

Description

This change adds unicode_literals, docstrings, and does some cleanup for the
worker code. I haven't done docstrings for the individual tools because I got
lazy.

Testing Done

Tested that workers still responded correctly.

Issues

Description	From	Last Updated
Also optional, I think.	mike_conley	Sept. 24, 2016, 12:58 p.m.
Missing env	mike_conley	Sept. 24, 2016, 12:59 p.m.
I think these are optional. At least, there are default args set.	mike_conley	Sept. 24, 2016, 12:59 p.m.
Can we call this queue_name ?	brennie	Oct. 12, 2016, 3:47 p.m.
"File"	brennie	Oct. 12, 2016, 3:47 p.m.
% (...) on next line	brennie	Oct. 12, 2016, 3:48 p.m.
Mind putting a blank line here?	chipx86	Oct. 25, 2016, 10:21 a.m.
And here.	chipx86	Oct. 25, 2016, 10:21 a.m.
"temporary"	chipx86	Oct. 25, 2016, 10:21 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:21 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:21 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:21 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:21 a.m.
"will only single line comment" might be missing a word or two.	chipx86	Oct. 25, 2016, 10:22 a.m.
Missing trailing period.	chipx86	Oct. 25, 2016, 10:21 a.m.
Missing a type.	chipx86	Oct. 25, 2016, 10:22 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:22 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:22 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:22 a.m.
Missing a type.	chipx86	Oct. 25, 2016, 10:23 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:23 a.m.
Missing a "Returns".	chipx86	Oct. 25, 2016, 10:24 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:24 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:24 a.m.
"bool"	chipx86	Oct. 25, 2016, 10:24 a.m.
:py:attr: is only for class attributes. There's no role for parameters. This should just use standard double backticks.	chipx86	Oct. 26, 2016, 1:54 p.m.
"bool"	chipx86	Oct. 25, 2016, 10:24 a.m.

Tool: PEP8 Style Checker
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py



Tool: Pyflakes
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py

Ship it!

bot/reviewbot/processing/review.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Also optional, I think.
```

bot/reviewbot/processing/review.py (Diff revision 1)

Should we be turning these TODO's into Splat bugs?

david

Sept. 24, 2016, 12:59 p.m.

No, I don't think so. Especially not at this stage of reviewbot.

bot/reviewbot/tools/process.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Missing env
```

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

The issue has been resolved. Show all issues

I think these are optional. At least, there are default args set.

Commit:

50d81e0edf2b61d00d61933b711706cce4cb31d7

033fb02561f8de20192390f245fc6a5137adeaa9

Diff:

Revision 2 (+396 -138)

Show changes

	bot/reviewbot/__init__.py
	bot/reviewbot/celery.py
	bot/reviewbot/tasks.py
	bot/reviewbot/utils.py
	bot/reviewbot/processing/filesystem.py
	bot/reviewbot/processing/review.py
	bot/reviewbot/tools/__init__.py
	bot/reviewbot/tools/buildbot.py
	6 more

Tool: Pyflakes
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py



Tool: PEP8 Style Checker
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py

Ship it!

bot/reviewbot/celery.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Can we call this queue_name ?
```
bot/reviewbot/processing/review.py (Diff revision 2)
The issue has been resolved. Show all issues
```
"File"
```

bot/reviewbot/processing/review.py (Diff revision 2)

The issue has been resolved. Show all issues

% (...) on next line

brennie

Oct. 12, 2016, 2:40 p.m.

Alternatively:

warning = (
    'WARNING: Number of comments exceeded, showing %d of %d.'
    % (max_comments, len(self.comments))
)

Commit:

033fb02561f8de20192390f245fc6a5137adeaa9

6db2e0102e99a7890f9f40ea55ec89138b4306b6

Diff:

Revision 3 (+402 -140)

Show changes

	bot/reviewbot/__init__.py
	bot/reviewbot/celery.py
	bot/reviewbot/tasks.py
	bot/reviewbot/utils.py
	bot/reviewbot/processing/filesystem.py
	bot/reviewbot/processing/review.py
	bot/reviewbot/tools/__init__.py
	bot/reviewbot/tools/buildbot.py
	6 more

Tool: Pyflakes
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py



Tool: PEP8 Style Checker
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py

bot/reviewbot/processing/filesystem.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Mind putting a blank line here?
```
bot/reviewbot/processing/filesystem.py (Diff revision 3)
The issue has been resolved. Show all issues
```
And here.
```
bot/reviewbot/processing/filesystem.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"temporary"
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"will only single line comment" might be missing a word or two.
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing trailing period.
```
bot/reviewbot/processing/review.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing a type.
```
bot/reviewbot/tasks.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/tasks.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/tasks.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/tools/__init__.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing a type.
```
bot/reviewbot/tools/__init__.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/tools/process.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Missing a "Returns".
```
bot/reviewbot/tools/process.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```
bot/reviewbot/tools/process.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```

bot/reviewbot/tools/process.py (Diff revision 3)

The issue has been resolved. Show all issues

"bool"

bot/reviewbot/utils.py (Diff revision 3)
The issue has been resolved. Show all issues
```
"bool"
```

Commit:

6db2e0102e99a7890f9f40ea55ec89138b4306b6

e13d365bd96aa025fe9fb0ff69716a006c3983b1

Diff:

Revision 4 (+411 -140)

Show changes

	bot/reviewbot/__init__.py
	bot/reviewbot/celery.py
	bot/reviewbot/tasks.py
	bot/reviewbot/utils.py
	bot/reviewbot/processing/filesystem.py
	bot/reviewbot/processing/review.py
	bot/reviewbot/tools/__init__.py
	bot/reviewbot/tools/buildbot.py
	6 more

Tool: Pyflakes
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py



Tool: PEP8 Style Checker
Processed Files:
    bot/reviewbot/tools/process.py
    bot/reviewbot/tasks.py
    bot/reviewbot/utils.py
    bot/reviewbot/tools/__init__.py
    bot/reviewbot/tools/pyflakes.py
    bot/reviewbot/tools/pep8.py
    bot/reviewbot/processing/filesystem.py
    bot/reviewbot/tools/cpplint.py
    bot/reviewbot/tools/buildbot.py
    bot/reviewbot/celery.py
    bot/reviewbot/tools/cppcheck.py
    bot/reviewbot/tools/jshint.py
    bot/reviewbot/__init__.py
    bot/reviewbot/processing/review.py

Ship it!

bot/reviewbot/tools/process.py (Diff revisions 3 - 4)

The issue has been resolved. Show all issues

:py:attr: is only for class attributes. There's no role for parameters. This should just use standard double backticks.

Status:: Completed
Change Summary:: Pushed to master (2f1c910)