Opened 6 years ago

Last modified 5 years ago

#5174 defect new

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

Reported by: Yaroslav Fedevych Owned by: Ken
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: Thijs Triemstra, Jonathan Jacobs Branch: branches/python-components-docs-5174
branch-diff, diff-cov, branch-cov, buildbot
Author: wirehead

Description (last modified by Thijs Triemstra)

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 Ken 5 years ago.
ticket-5174-2-docs.patch (5.4 KB) - added by Ken 5 years ago.
v2 patch, fixed a few more things.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by Thijs Triemstra

Cc: Thijs Triemstra added
Description: modified (diff)
Keywords: documentation added

comment:2 Changed 5 years ago by Ken

Owner: set to Ken
Status: newassigned

Changed 5 years ago by Ken

Attachment: ticket-5174-docs.patch added

comment:3 Changed 5 years ago by Ken

Keywords: review added
Owner: Ken deleted
Status: assignednew

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 5 years ago by Stephen Thorne

Keywords: review removed

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

comment:5 Changed 5 years ago by Stephen Thorne

Owner: set to Ken

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

Changed 5 years ago by Ken

Attachment: ticket-5174-2-docs.patch added

v2 patch, fixed a few more things.

comment:6 Changed 5 years ago by Ken

Keywords: review added
Owner: Ken deleted

comment:7 Changed 5 years ago by Jonathan Jacobs

Author: jonathanj
Branch: branches/python-components-docs-5174

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

comment:8 Changed 5 years ago by Jonathan Jacobs

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

comment:9 Changed 5 years ago by Jonathan Jacobs

Author: jonathanjwirehead
Keywords: review removed
Owner: set to Ken

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 5 years ago by Jonathan Jacobs

Cc: Jonathan Jacobs added
Note: See TracTickets for help on using tickets.