Opened 11 months ago

Last modified 45 minutes ago

#7013 enhancement new

twisted.python.threadpool should comply with whitespace and coding standards

Reported by: hawkowl Owned by: hawkowl
Priority: normal Milestone:
Component: core Keywords: documentation review
Cc: Branch: branches/threadpool-coding-standards-7013
(diff, github, buildbot, log)
Author: hawkowl Launchpad Bug:


#7011 made me see that there could be some improvements in here.

Change History (5)

comment:1 Changed 11 months ago by hawkowl

  • Author set to hawkowl
  • Branch set to branches/threadpool-coding-standards-7013

(In [41789]) Branching to threadpool-coding-standards-7013.

comment:2 Changed 11 months ago by hawkowl

  • Keywords documentation review added
  • Owner hawkowl deleted

Putting it up for review - hopefully someone with more knowledge of ThreadPool can make sure that my documentation is correct (I've been reading the code and I think it's right, but that's what review is for!)

comment:3 Changed 11 months ago by adiroiban

  • Keywords review removed
  • Owner set to hawkowl

Thanks for the changes. Look good. Only minor comments while I exercise my review skills :)

# Create enough, but not too many

Not your code, but now that we are on it, do we need this comment since the docstring already talks about it.

log.msg('queue: %s'   % (self.q.queue,))

Just asking since this about coding standards. Is this the coding convention ? I was expecting:

log.msg('queue: %s' % (self.q.queue,))

Not sure if stopAWorker is stopping the next working that is finnished.... maybe WorkerStop marker should be renamed to StopNextFinnishedWorker ... or add documentation for WorkerStop

Other than that, all is good. Thanks!

Last edited 45 minutes ago by adiroiban (previous) (diff)

comment:4 Changed 50 minutes ago by hawkowl

Thanks, Adi.

I fixed up that little spacing issue, and I'm forcing a build now.

comment:5 Changed 47 minutes ago by hawkowl

  • Keywords review added

Okay, builds are passing.

It's now TwistedChecker and Pyflakes clean, except for TwistedChecker bugs (dict is a valid thing, twistedchecker!) and a scoping issue in a while block that pyflakes doesn't seem to think is being used afterwards, although the next while block will use it, maybe. It's a valid warning, because it's a bit icky, but isn't really in the scope of this ticket. There's also green across the board with buildbots so far.

Putting up for review, in case someone with ThreadPool knowledge specifically can say that the docstrings are correct.

Note: See TracTickets for help on using tickets.