Issue 3703 - Add "coin flip" feature to abpbot

Issue 3703 - Add "coin flip" feature to abpbot

[kzar, 2016-02-25 19:53:21]

Patch Set 1
Patch Set 2 : Ensure IRC bot username is at the beginning of a question string

[Sebastian Noack, 2016-02-25 20:09:42]

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py
File decisionbot.py (right):

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py#newc...
decisionbot.py:2: # coding: utf-8
Nit: We recently decided to not use coding declarations in new code for PEP-8
compliance.

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py#newc...
decisionbot.py:4: import random
Nit; also from PEP-8: Imports are always put at the top of the file, just after
any module comments and docstrings, and before module globals and constants.

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py#newc...
decisionbot.py:24: self.question_regexp =
re.compile("^%s:?(.+\s+or\s+.+)\?+\s*$" %
Perhaps we should use the case-insensitive flag for this regexp. Same below.

[kzar, 2016-02-25 20:29:51]

Patch Set 3 : Addressed feedback

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py
File decisionbot.py (right):

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py#newc...
decisionbot.py:2: # coding: utf-8
On 2016/02/25 20:09:42, Sebastian Noack wrote:
> Nit: We recently decided to not use coding declarations in new code for PEP-8
> compliance.

Done.

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py#newc...
decisionbot.py:4: import random
On 2016/02/25 20:09:42, Sebastian Noack wrote:
> Nit; also from PEP-8: Imports are always put at the top of the file, just
after
> any module comments and docstrings, and before module globals and constants.

Done.

https://codereview.adblockplus.org/29337738/diff/29337742/decisionbot.py#newc...
decisionbot.py:24: self.question_regexp =
re.compile("^%s:?(.+\s+or\s+.+)\?+\s*$" %
On 2016/02/25 20:09:42, Sebastian Noack wrote:
> Perhaps we should use the case-insensitive flag for this regexp. Same below.

Done.

[Sebastian Noack, 2016-02-25 20:47:53]

https://codereview.adblockplus.org/29337738/diff/29337745/decisionbot.py
File decisionbot.py (right):

https://codereview.adblockplus.org/29337738/diff/29337745/decisionbot.py#newc...
decisionbot.py:10: import random
Nit: Please add an empty line between the docstring and imports.

https://codereview.adblockplus.org/29337738/diff/29337745/decisionbot.py#newc...
decisionbot.py:21: self.question_regexp =
re.compile("^%s:?(.+\s+or\s+.+)\?+\s*$" %
I just realized that you don't use r"" strings. But for some reason is "\s" ==
"\\s" == r"\s". However, I think it's still better to use r"".

[kzar, 2016-02-25 20:52:40]

Patch Set 4 : Fixed a couple more nits

https://codereview.adblockplus.org/29337738/diff/29337745/decisionbot.py
File decisionbot.py (right):

https://codereview.adblockplus.org/29337738/diff/29337745/decisionbot.py#newc...
decisionbot.py:10: import random
On 2016/02/25 20:47:53, Sebastian Noack wrote:
> Nit: Please add an empty line between the docstring and imports.

Done.

https://codereview.adblockplus.org/29337738/diff/29337745/decisionbot.py#newc...
decisionbot.py:21: self.question_regexp =
re.compile("^%s:?(.+\s+or\s+.+)\?+\s*$" %
On 2016/02/25 20:47:53, Sebastian Noack wrote:
> I just realized that you don't use r"" strings. But for some reason is "\s" ==
> "\\s" == r"\s". However, I think it's still better to use r"".

Done.

[Sebastian Noack, 2016-02-25 21:04:26]

LGTM. But don't land before either fhd or palant gave their OK, that we actually
want to have that feature, in there.