-
Notifications
You must be signed in to change notification settings - Fork 19
Bash module #89
base: master
Are you sure you want to change the base?
Bash module #89
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few questions, comments, and requests inline. Please also fix this up to match the PEP8 formatting standards.
Since this relies on a web service, I would also like to see some tests here. Many of the existing modules already break on a regular basis due to web service API changes and I'd prefer to at least catch this if it happens.
'key': phenny.config.bash_api_key | ||
} | ||
|
||
web.post('https://bash.vtluug.org/quotes', {}, {}, True, json=quote_json) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this to a config option.
@@ -0,0 +1,296 @@ | |||
#!/usr/bin/python3 | |||
""" | |||
bash.py - bash.org clone. Saves quotes from quote-logger module to a db |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not call this pyqdb? Calling it bash is confusing if it doesn't have API compatibility with bash.org.
global queue | ||
queue.append(sqlite_data) | ||
|
||
def bash(phenny, input): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand how to use this at all. Can you please improve the documentation? Maybe add an example?
|
||
# Check Input Type | ||
if not nick1_start_str.isdigit() or not nick2_end_str.isdigit(): | ||
phenny.say('Error: 2nd & 4th argument must be integers') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a user-friendly error message.
nick2_total = nick2_total + row[0] | ||
|
||
if nick1_total < nick1_start: | ||
phenny.say("Error: {} has not done {} actions ({} replies not included)" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error message is not user-friendly.
|
||
bash.conn = None | ||
bash.rule = (['bash', 'vtbash', 'quote'], r'(.*)') | ||
logger.event = '*' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a typo, or is it supposed to be like this? If it's supposed to be like this, can you please add a comment explaining why?
I'm an idiot and forgot how pull requests work so this is just a continuation of #75