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

Quote plugin #115

Closed
wants to merge 7 commits into from
Closed

Quote plugin #115

wants to merge 7 commits into from

Conversation

JohnBee
Copy link

@JohnBee JohnBee commented Oct 1, 2016

  • Moved to MongoDB
  • Simpler interface only requiring nick to add and recall.

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.

Much better, IMO :)

Did the old PRQ need closing though? I'd have preferred a rebase (you'll have to do this at some point to get my test fixes from #114 into your tree so the tests pass)

Writing tests is the next step :)

raise self.QuoteError("No nick!")

quote = {} #output
quote["quote"] = last.last_message(nick)[2]
Copy link
Member

@LordAro LordAro Oct 2, 2016

Choose a reason for hiding this comment

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

I would be very surprised if this worked at all... Plugins aren't generally good at interacting with each other like that. Have you tested this?

def _quoteExists(self, quote_post):
"""Checks to see if a nick + quote already exists to stop repeats."""
if self.db.find(quote_post).count > 0:
return True
Copy link
Member

Choose a reason for hiding this comment

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

Come on, boolean logic please :)

else:
return False

def _addquote(self, nick=""):
Copy link
Member

Choose a reason for hiding this comment

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

Why use a default parameter if you just error on it anyway? Better to have a TypeError if you manage to call without a nick, imo

@coveralls
Copy link

coveralls commented Oct 30, 2016

Coverage Status

Coverage decreased (-1.4%) to 68.024% when pulling 07be5d1 on JohnBee:master into 2fb243e on HackSoc:master.

@LordAro
Copy link
Member

LordAro commented Sep 26, 2017

Superseded by #123

@LordAro LordAro closed this Sep 26, 2017
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