[Merge] lp:~stgraber/upstart/upstart-session-socket into lp:upstart

James Hunt james.hunt at canonical.com
Wed Jan 16 15:01:26 UTC 2013


* conf/rc-sysinit.conf: I've got some reservations about changing the
  default jobs we provide. That said, those building systems should
  really be creating their own jobs. I think I'd be happy if we added a
  note to the README stating that the files in 'conf/' are examples only
  and *will* need to be changed by those creating systems using Upstart.
* conf/rc.conf: As above.
* init/control.c: control_init(): Spaces before '(' (4 occurences).
* init/job_process.c:
* init/main.c:
* util/initctl.c:
  - user_mode: Comment should be something like 'if TRUE, talk to
    Upstart over a private D-Bus socket'.
  - upstart_open():
    - Space before '(' of getenv call (2 occurences).
    - I'd be tempted to save the value of the first call to getenv to
      avoid the 2nd call.
    - 'else' should be on same line as closing brace of 'if'.
    - Shouldn't the logic be inverted here to conform to?:
      https://wiki.ubuntu.com/FoundationsTeam/Specs/RaringUpstartUserSessions#Effect_of_UPSTART_SESSION
      So, by default, if UPSTART_SESSION is set, use the private D-Bus
      socket, unless the user specifies '--system'.
  - options: I'd be tempted to change 'start' to 'run' as the former
    implies a long-running operation.
* init/man/init.5: 'Job environment' section needs to be updated for
  'UPSTART_SESSION'.
* util/man/initctl.8: Need to document '--user'.
* util/tests/test_initctl.c: We could do with a test for '--user'. This
  will necessitate something like:
  - Changing _START_UPSTART() into a function and allowing atleast 1 extra
    argument to be specified or specifying the type (meaning '--user' or
    '--session') would work, along with a test 
  - Changing WAIT_FOR_UPSTART() into a function to allow the session
    bus or private socket to be connected to.
  - Then, a test to simply start upstart, wait for it, create a conf
    file, check that initctl list shows that job, then stop upstart
    (see test_list()).
-- 
https://code.launchpad.net/~stgraber/upstart/upstart-session-socket/+merge/143344
Your team Upstart Reviewers is requested to review the proposed merge of lp:~stgraber/upstart/upstart-session-socket into lp:upstart.



More information about the upstart-devel mailing list