Ticket #3197 defect closed fixed

Opened 5 years ago

Last modified 5 years ago

IProcessTransport.pid is incompletely documented

Reported by: exarkun Owned by:
Priority: normal Milestone:
Component: core Keywords: documentation
Cc: thijs Branch: branches/pid-doc-3197-2
(diff, github, buildbot, log)
Author: thijs, exarkun Launchpad Bug:

Description

After a process ends, its pid attribute is set to None. The interface doesn't document this, or the type of pid while the process is running.

Attachments

processtransport-pid.patch Download (0.7 KB) - added by exarkun 5 years ago.

Change History

1

  Changed 5 years ago by thijs

  • status changed from new to assigned
  • keywords documentation added
  • cc thijs added
  • owner changed from glyph to thijs

2

  Changed 5 years ago by thijs

  • branch set to branches/pid-doc-3197
  • author set to thijs

(In [24893]) Branching to 'pid-doc-3197'

3

  Changed 5 years ago by thijs

  • keywords documentation, review added; documentation removed
  • owner thijs deleted
  • status changed from assigned to new

Added the None docstring in r24894, but still need to know the 'type of pid while the process is running'.

4

  Changed 5 years ago by exarkun

  • keywords documentation added; documentation, review removed
  • owner set to thijs

It's an int while it's not None. It's worth mentioning that it is set to None before processEnded is called, I think.

5

  Changed 5 years ago by thijs

  • status changed from new to assigned

6

  Changed 5 years ago by thijs

  • status changed from assigned to new
  • owner thijs deleted
  • keywords documentation, review added; documentation removed

Updated the docstring in r25029. I manually merged forward, creating a new branch is too much for such a small change imo.

7

  Changed 5 years ago by exarkun

Small change, but it conflicts if I try to merge it with trunk. I'm not sure what a "manual merge forward" is, but since it doesn't prevent conflicts I think this needs a new branch, or at least attach a patch with the resolved diff to the ticket. Actually, I'll do that.

Changed 5 years ago by exarkun

8

  Changed 5 years ago by exarkun

  • author changed from thijs to thijs, exarkun

9

follow-up: ↓ 12   Changed 5 years ago by glyph

  • keywords documentation added; documentation, review removed
  • owner set to exarkun

This is an improvement, it should be merged. It's a bit awkwardly worded though - it implies that after processEnded it called, pid is set to something different.

10

  Changed 5 years ago by thijs

  • status changed from new to assigned
  • owner changed from exarkun to thijs

11

  Changed 5 years ago by thijs

  • branch changed from branches/pid-doc-3197 to branches/pid-doc-3197-2

(In [25316]) Branching to 'pid-doc-3197-2'

12

in reply to: ↑ 9   Changed 5 years ago by thijs

  • status changed from assigned to new
  • owner changed from thijs to exarkun

Replying to glyph:

This is an improvement, it should be merged. It's a bit awkwardly worded though - it implies that after processEnded it called, pid is set to something different.

That's what it implies but what should it say otherwise?

Created a new branch and applied the patch. Assigning it to exarkun so he can possibly rephrase it and then merge.

13

  Changed 5 years ago by exarkun

(In [25324]) another approach

refs #3197

14

  Changed 5 years ago by exarkun

(In [25325]) s/C{/L{/

refs #3197

15

  Changed 5 years ago by exarkun

  • keywords review added
  • owner exarkun deleted

16

  Changed 5 years ago by radix

  • owner set to exarkun
  • keywords review removed

I thought we don't use L{} for built-in types and None?

If we do, then merge. If we don't, change to C{} and merge.

17

  Changed 5 years ago by exarkun

Lately, I feel like L{} means fully-qualified Python name and C{} means evaluatable-ish expression. So I guess I'll leave it as-is.

18

  Changed 5 years ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(In [25367]) Merge pid-doc-3197-2

Author: thijs, exarkun Reviewer: radix Fixes: #3197

Expand the interface documentation for IProcessTransport.pid.

19

  Changed 2 years ago by <automation>

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