search

ltworf / ltdata

IRC bot in python
2 stars 2 forks source link

React: Don't stop after the first match #4

Closed salto-edit closed 9 years ago

ltworf commented 9 years ago

@vlopezj how will our users react to the change in interface, given that the bot will start to react to multiple reacts after this change?

vlopezj commented 9 years ago

I think they can deal with it :)

ltworf notifications@github.com skrev: (13 maj 2015 16:53:54 CEST)

@vlopezj how will our users react to the change in interface, given that the bot will start to react to multiple reacts after this change?

Reply to this email directly or view it on GitHub: https://github.com/ltworf/ltdata/pull/4#issuecomment-101701163

ltworf commented 9 years ago

Hm, are you sure it's a good idea? Maybe they will be confused and will not know what is going on.

vlopezj commented 9 years ago

They can ask for a refund if they don't like it.

ltworf notifications@github.com skrev: (13 maj 2015 17:14:18 CEST)

Hm, are you sure it's a good idea? Maybe they will be confused and will not know what is going on.

Reply to this email directly or view it on GitHub: https://github.com/ltworf/ltdata/pull/4#issuecomment-101711655

salto-edit commented 9 years ago

I fixed the issues mentioned in your line comments. Please merge.

ltworf commented 9 years ago

@salto-edit thanks for the fixes!

The list is needed instead of a tuple because tuples are not mutable and hashable objects in Python, while lists are mutable (and unhashable).

As a more stylistic note, for the next time it would have been better to have just 2 commits with the proper initializations, you could use git rebase to do that.

I'm going to merge and test this.

salto-edit commented 9 years ago

Sure, but won't that mess up the pull request? Maybe not... will try next time.

ltworf commented 9 years ago

You need to rewrite history with a push -f :) But be careful not to destroy everything when you do it.