← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~jameinel/launchpad/lp-service into lp:launchpad

 

Review: Approve
As I said on IRC, I think you should wait until the daemon process is accepting requests before proceeding in _start_subprocess.  

The 'out' variable in _daemonize is strangely located and named -- can you call it something clearer and assign to it closer to its use?  I don't think we need to chdir to /, so please delete that comment.  I don't think we really need to setsid either, but it does no harm I guess.

It's a bit of a shame you had to write (or copy/paste) so much code for the process handling, but oh well.  There are good reasons this daemon isn't using twistd.
-- 
https://code.launchpad.net/~jameinel/launchpad/lp-service/+merge/40386
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/launchpad/lp-service into lp:launchpad.



References