Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iou plugin #160

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

iou plugin #160

wants to merge 2 commits into from

Conversation

FromAnkyra
Copy link
Contributor

iou and iamowed commands, using iou plugin

updating cause I forked this ages ago
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.8%) to 70.603% when pulling 0d6d2bb on FromAnkyra:master into b62c1fd on HackSoc:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.8%) to 70.603% when pulling 0d6d2bb on FromAnkyra:master into b62c1fd on HackSoc:master.

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An excellent start! No, really, there's little to no documentation for doing this, so it's not the easiest thing in the world

Some tests would be excellent, and mind the codestyle in a few places - when to put spaces around operators, particularly (A plugin for whatever editor you're using really helps with this!)

@Plugin.command('iou', help = ('equivalent to IOU'))
def iou(self, e):
nick_ = nick(e['user']).strip('<')
nick_ = nick_.strip('>')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nick would never contain these characters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure? I did this when trying to fix iamowed, where

!iou a b n
!iamowed b a 

always returned 0 instead of n, and after a bit of poking it seemed that the reason was cause e['user'] gave the name in <>. I may be wrong, but in that case there's probably something else majorly wrong, so it would be good to know for sure. (By which I mean I'll check, but if you know it would be useful, and also won't fix immediately)

Copy link
Member

@LordAro LordAro Jun 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, there is some weirdness with e['user'] - i notice csbot.util.nick is used in almost every place that e['user'] is, though that only splits on '!'. Should probably use that regardless, rather than the strips

src/csbot/plugins/iou.py Outdated Show resolved Hide resolved
src/csbot/plugins/iou.py Outdated Show resolved Hide resolved
src/csbot/plugins/iou.py Outdated Show resolved Hide resolved
src/csbot/plugins/iou.py Outdated Show resolved Hide resolved
src/csbot/plugins/iou.py Outdated Show resolved Hide resolved

self.blankuser = {"_id": "example", "owee_nick": 0}

x = self.db_owage.insert(self.blankuser)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this setup stuff necessary? I don't see it being done in the other mongodb plugins

src/csbot/plugins/iou.py Outdated Show resolved Hide resolved
def update_owage(self, nick, owee_name, amount):
all_owed = self.db_owage.find_one(nick)
if all_owed:
self.db_owage.delete_one(all_owed)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps using update_one would be better than finding, deleting, inserting, then finding again

src/csbot/plugins/iou.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants