Opened 12 years ago
Closed 12 years ago
#3910 defect closed fixed (fixed)
twisted.words.protocols.irc: Don't prepend # onto channel name in IRCClient.me
Reported by: | stefanor | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | words | Keywords: | |
Cc: | Branch: | ||
Author: |
Description
You can't send an action privately (not in a channel) using IRCClient
, because IRCClient.me
prepends a #
onto the channel name. While that behaviour is documented in docstrings elsewhere (See ticket #3895), it is a bad API choice.
Without this patch, the alternative is to call IRCClient.ctcpMakeQuery
directly.
Attachments (2)
Change History (10)
comment:1 follow-up: 2 Changed 12 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to stefanor |
Changed 12 years ago by
Attachment: | t.w.p.irc-action.patch added |
---|
describe() function replaces me() and doesn't prepend #
comment:2 Changed 12 years ago by
Keywords: | review added |
---|
Replying to exarkun:
Instead, a new method with the desired behavior should be introduced, and possibly the existing method should be deprecated.
All the good names are taken. the best I can come up with is describe()
Please be sure to include unit tests along with any change. :)
Bleh, OK.
comment:3 Changed 12 years ago by
Owner: | changed from stefanor to Jean-Paul Calderone |
---|
comment:4 follow-up: 5 Changed 12 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to stefanor |
Thanks for the new patch. I just have a few comments:
- The new test method,
test_describe
, should have a docstring describing what behavior it covers. You can see examples of such docstrings just above it on the other test methods in that class. - The Twisted coding standard calls for two blank lines between method definitions; please add a blank line before
test_describe
and afterdescribe
. - The
me
deprecation should also be unit tested. Unfortunately this is pretty old code, some
doesn't have any existing unit tests. Also unfortunately, Twisted hasn't quite settled on a convention for testing deprecations, so there are a few possibilities for you to choose from here. :) One style involvesTestCase.flushWarnings
, you can find an example of this style in test_util.py. Another style involvesTestCase.callDeprecated
, and you can find an example in test_keys.py (this involves a different implementation of emitting the warning too, see asn1.py. There's alsoTestCase.assertWarns
, but either of the previous forms is probably preferable to that. - The deprecation should mention that the API was first deprecated in Twisted 9.0. Depending on how you resolve the previous point, this may be fixed automatically.
- The new
describe
method should include the epytext markup@since: 9.0
in its docstring.
That's all. Thanks again for working on this.
Changed 12 years ago by
Attachment: | t.w.p.irc-action-2.patch added |
---|
Improve tests and test deprecation
comment:5 Changed 12 years ago by
Keywords: | review added |
---|---|
Owner: | changed from stefanor to Jean-Paul Calderone |
Replying to exarkun:
Thanks again for working on this.
OK, I admit I was lazier in the earlier patch attempts :P
Hope this solves all the issues.
comment:7 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:8 Changed 10 years ago by
Owner: | Jean-Paul Calderone deleted |
---|
This is what the API does. Changing it may break existing software which relies on it.
Instead, a new method with the desired behavior should be introduced, and possibly the existing method should be deprecated.
Please be sure to include unit tests along with any change. :)