Opened 4 years ago

Closed 4 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
(diff, github, buildbot, log)
Author: taos Launchpad Bug:

Description

I think there should be a invite method to IRCClient.

Attachments (2)

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

Download all attachments as: .zip

Change History (14)

Changed 4 years ago by taos

Changed 4 years ago by taos

comment:1 Changed 4 years ago by taos

  • Keywords review added
  • Owner exarkun deleted

comment:2 Changed 4 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/ircclient-invite-4820

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

comment:3 Changed 4 years ago by exarkun

(In [30578]) Apply invite.patch

refs #4820

comment:4 Changed 4 years ago by exarkun

  • Author changed from exarkun to taos

comment:5 Changed 4 years ago by exarkun

  • Keywords review removed
  • Owner set to exarkun
  • Status changed from new to assigned

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 4 years ago by exarkun

(In [30580]) vertical whitespace fixes

refs #4820

comment:7 Changed 4 years ago by exarkun

(In [30581]) @since marker

refs #4820

comment:8 Changed 4 years ago by exarkun

(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 4 years ago by exarkun

(In [30583]) Remove the redundant test_invite

refs #4820

comment:10 Changed 4 years ago by exarkun

(In [30584]) restore deleted blank line

refs #4820

comment:11 Changed 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from assigned to closed

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

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

Add IRCClient.invite method to send INVITE messages.

comment:12 Changed 3 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.