Opened 7 years ago

Closed 7 years ago

#4820 enhancement closed fixed (fixed)

Adding invite to IRCClient

Reported by: taos Owned by:
Priority: normal Milestone:
Component: words Keywords:
Cc: Branch: branches/ircclient-invite-4820
branch-diff, diff-cov, branch-cov, buildbot
Author: taos

Description

I think there should be a invite method to IRCClient.

Attachments (2)

invite.patch (2.2 KB) - added by taos 7 years ago.
4820.feature (64 bytes) - added by taos 7 years ago.

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by taos

Attachment: invite.patch added

Changed 7 years ago by taos

Attachment: 4820.feature added

comment:1 Changed 7 years ago by taos

Keywords: review added
Owner: Jean-Paul Calderone deleted

comment:2 Changed 7 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/ircclient-invite-4820

(In [30577]) Branching to 'ircclient-invite-4820'

comment:3 Changed 7 years ago by Jean-Paul Calderone

(In [30578]) Apply invite.patch

refs #4820

comment:4 Changed 7 years ago by Jean-Paul Calderone

Author: exarkuntaos

comment:5 Changed 7 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Jean-Paul Calderone
Status: newassigned

Thanks for working on this patch. The implementation looks straightforward and correct to me. Concerning the tests though:

  1. I like ClientInviteTests, except for
    1. the lack of docstrings
    2. the testFoo naming (instead of the preferred test_foo style).
  2. Oops, also, the setUp call in testInvite isn't necessary. I think ClientTests.test_invite is probably redundant with the tests on ClientInviteTests though.
  3. methods should be separated from each other by two blank lines; classes by three.
  4. New APIs (like IRCClient.invite) should be marked up with @since tags

These are simple points, so I'll go ahead and correct them and then merge the change. Thanks again!

comment:6 Changed 7 years ago by Jean-Paul Calderone

(In [30580]) vertical whitespace fixes

refs #4820

comment:7 Changed 7 years ago by Jean-Paul Calderone

(In [30581]) @since marker

refs #4820

comment:8 Changed 7 years ago by Jean-Paul Calderone

(In [30582]) Add docstrings to everything on ClientInviteTests; fix vertical whitespace and test method naming; remove spurious setUp call; insert missing whitespace between function arguments.

refs #4820

comment:9 Changed 7 years ago by Jean-Paul Calderone

(In [30583]) Remove the redundant test_invite

refs #4820

comment:10 Changed 7 years ago by Jean-Paul Calderone

(In [30584]) restore deleted blank line

refs #4820

comment:11 Changed 7 years ago by Jean-Paul Calderone

Resolution: fixed
Status: assignedclosed

(In [30585]) Merge ircclient-invite-4820

Author: taos, exarkun Reviewer: exarkun Fixes: #4820

Add IRCClient.invite method to send INVITE messages.

comment:12 Changed 7 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.