Opened 3 years ago

Last modified 16 months ago

#5174 defect new

The docstring of twisted.python.components.Componentized.addComponent is not consistent with its code

Reported by: jafd Owned by: wirehead
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: thijs, jonathanj Branch: branches/python-components-docs-5174
(diff, github, buildbot, log)
Author: wirehead Launchpad Bug:

Description (last modified by thijs)

This docstring contains the following:

  @return: the list of appropriate interfaces

While the code itself does not have any return statement at all.

The only instance of code invoking addComponent, at twisted.application.service.Application, does not expect any return value.

Either docstring should be fixed to not mention return values, or the code should actually return it.

Attachments (2)

ticket-5174-docs.patch (559 bytes) - added by wirehead 16 months ago.
ticket-5174-2-docs.patch (5.4 KB) - added by wirehead 16 months ago.
v2 patch, fixed a few more things.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 3 years ago by thijs

  • Cc thijs added
  • Description modified (diff)
  • Keywords documentation added

comment:2 Changed 16 months ago by wirehead

  • Owner set to wirehead
  • Status changed from new to assigned

Changed 16 months ago by wirehead

comment:3 Changed 16 months ago by wirehead

  • Keywords review added
  • Owner wirehead deleted
  • Status changed from assigned to new

This doc issue annoyed me. I took a peek and there has never been, in the past decade, a point at which addComponent returned something... so it seems simpler to just remove the trick documentation instead of fixing the code.

comment:4 Changed 16 months ago by jerub

  • Keywords review removed

Please also document the parameters, component and ignoreClass. They're documented in prose but need @param annotations.

comment:5 Changed 16 months ago by jerub

  • Owner set to wirehead

Please also document the parameters, component and ignoreClass. They're documented in prose but need @param annotations.

Changed 16 months ago by wirehead

v2 patch, fixed a few more things.

comment:6 Changed 16 months ago by wirehead

  • Keywords review added
  • Owner wirehead deleted

comment:7 Changed 16 months ago by jonathanj

  • Author set to jonathanj
  • Branch set to branches/python-components-docs-5174

(In [37892]) Branching to 'python-components-docs-5174'

comment:8 Changed 16 months ago by jonathanj

(In [37896]) Fix some minor grammatical errors, refs #5174.

comment:9 Changed 16 months ago by jonathanj

  • Author changed from jonathanj to wirehead
  • Keywords review removed
  • Owner set to wirehead

Thank you for improving the documentation in Twisted, in particular removing the use of a first-person narrative!

Some issues I noticed with your patch:

  1. Some docstrings contain annotations like #param ..., I've looked through the epytext syntax specification and I can't find what this means, I suspect it may just be a typo of @param ....
  1. When your epytext field (such as @param) is wrapped over multiple lines, the wrapped lines (that is, from the second line onwards) need to be indented. See the docstring for proxyForInterface for an example.
  1. Some field annotations do not begin with a capital letter and/or end with a period.
  1. The text *Twisted* will appear literally as-is, if you are trying to italicize text you need to use I{Twisted} or B{Twisted} for bold.
  1. Some @param fields have been adapted from more general text but still refer to themselves in the third person, addAdapter's ignoreClass parameter is one such example.
  1. There are a few Twisted coding standard violations that were either introduced or were already present:
  1. Docstring opening and closing triple quotes should appear on lines by themselves.
  1. There should be 3 new lines between class definitions and 2 new lines between method definitions.
  1. The Twisted checker buildbot happens to contain a list of newly introduced twisted checker warnings, please fix these. Although there are some existing checker issues that should be fixed too:
  1. Methods should document their return value with @return.

I also took the liberty of creating a branch and applying your patch, as well as fixing some minor grammatical errors, please base future diffs on this branch.

comment:10 Changed 16 months ago by jonathanj

  • Cc jonathanj added
Note: See TracTickets for help on using tickets.