Opened 5 years ago

Closed 5 years ago

#4474 enhancement closed fixed (fixed)

ListOf AMP Arguments should support 'optional' flag

Reported by: faldridge Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

Like all other AMP Argument subclasses, t.p.amp.ListOf should allow callers to pass 'optional=True' to its init() method, thereby allowing the argument to be omitted from the protocol.

Attached is a patch that provides the correct behavior with the necessary update to ListOf's docstring.

As the functionality is handled by the base class (t.p.amp.Argument), and not individual subclasses, and as the functionality already has unit tests, there are no unit tests included in this patch.

Attachments (1)

listof-optional-4474.patch (3.1 KB) - added by faldridge 5 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 follow-up: Changed 5 years ago by exarkun

  • Keywords review removed
  • Owner changed from glyph to faldridge

The patch looks good. Thanks. A couple things though:

  1. In @foo docs, after the first line, lines should be indented 4 spaces.
  2. The feature should have unit tests.

Changed 5 years ago by faldridge

comment:2 in reply to: ↑ 1 Changed 5 years ago by faldridge

  • Keywords review added
  • Owner changed from faldridge to exarkun

Replying to exarkun:

The patch looks good. Thanks. A couple things though:

  1. In @foo docs, after the first line, lines should be indented 4 spaces.
  2. The feature should have unit tests.
  1. Done.
  2. Added.

comment:3 Changed 5 years ago by exarkun

  • Keywords review removed

Thanks, looks good, I'll apply (including a news fragment).

comment:4 Changed 5 years ago by exarkun

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

(In [29159]) Add optional parameter to twisted.protocols.amp.ListOf

Author: faldridge
Reviewer: exarkun
Fixes: #4474

Let ListOf arguments be optional in the usual way.

comment:5 Changed 4 years ago by <automation>

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