lalita

Merge lp://staging/~mdione/lalita/url-plugin into lp://staging/lalita

  1. url-plugin
  2. Merge into trunk
Proposed by Marcos Dione
Status: Merged
Merged at revision: 177
Proposed branch: lp://staging/~mdione/lalita/url-plugin
Merge into: lp://staging/lalita
Diff against target: 236 lines (+53/-30)
2 files modified
lalita/ircbot.py (+11/-5)
lalita/plugins/url.py (+42/-25)
To merge this branch: bzr merge lp://staging/~mdione/lalita/url-plugin
Reviewer Review Type Date Requested Status
Facundo Batista Approve
Review via email: mp+164588@code.staging.launchpad.net

Description of the change

fixes bug #1181506.

To post a comment you must log in.
lp://staging/~mdione/lalita/url-plugin updated
175. By Facundo Batista

Delayed registration.

176. By Facundo Batista

Deferred registration is now a config for Freenode.

Revision history for this message
Facundo Batista (facundo) wrote :

Some details:

- When logging, use the safe way to build strings... like this (not using '%'):

    logger.debug('command_char: %s', self.command_char)

- Don't log 'msg' *again* in the cases of "found he's talking to me" and "command char found" (no point of having a bigger log and more difficult to read if the info is already there)

- Revert the change in seen.py (the separation for the class should be two lines, and there's no point of touching a file in the history only for a blank change).

- Try to be pep 8, please!

review: Needs Fixing
Revision history for this message
Facundo Batista (facundo) wrote :

This is wrong (in ircbot.py):

  logger.debug("plugin %s, loglevel %s", (plugin, loglvl))

You're logging a tuple for the first %s and failing to find something for the second one...

Also, you're just adding whitespace (that shouldn't be there) in seen.py, please revert that.

Thanks!

review: Needs Fixing
lp://staging/~mdione/lalita/url-plugin updated
177. By Facundo Batista

* implement ALLOWED_PORTS. should move to a config file option.
* fixes #1181506.
* read URLs from ACTIONs too.
* support for old/broken BS.

Revision history for this message
Facundo Batista (facundo) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: