Bluetooth Menu

Merge lp://staging/~charlesk/indicator-bluetooth/upstart-job into lp://staging/indicator-bluetooth/14.04

  1. upstart-job
  2. Merge into trunk.14.04
Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 80
Merged at revision: 78
Proposed branch: lp://staging/~charlesk/indicator-bluetooth/upstart-job
Merge into: lp://staging/indicator-bluetooth/14.04
Diff against target: 117 lines (+67/-14)
5 files modified
data/Makefile.am (+41/-11)
data/indicator-bluetooth.conf.in (+11/-0)
data/indicator-bluetooth.desktop.in (+7/-0)
data/indicator-bluetooth.service.in (+0/-3)
data/indicator-bluetooth.upstart.desktop.in (+8/-0)
To merge this branch: bzr merge lp://staging/~charlesk/indicator-bluetooth/upstart-job
Reviewer Review Type Date Requested Status
Iain Lane (community) desktop-files Approve
PS Jenkins bot (community) continuous-integration Approve
Ted Gould (community) Approve
Review via email: mp+198100@code.staging.launchpad.net

Commit message

Switching DBus service to an Upstart Job.

Description of the change

Looks like this indicator was overlooked back when all the others got switched over to upstart.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:77
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/28/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-trusty-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-trusty-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-trusty-armhf-ci/5/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-bluetooth-ci/28/rebuild

review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

Instead of NotShowIn, you should ship a desktop file with Hidden=true in /usr/share/upstart/xdg/autostart/ (this directory is prepended to XDG_CONFIG_DIRS under upstart user sessions). This will cause it to not start via XDG autostart exactly when user sessions are in use, which is probably what you really want. Having Unity in there smells a bit.

review: Needs Fixing
Revision history for this message
Ted Gould (ted) wrote :

On Fri, 2013-12-06 at 17:44 +0000, Iain Lane wrote:

> Instead of NotShowIn, you should ship a desktop file with Hidden=true in /usr/share/upstart/xdg/autostart/ (this directory is prepended to XDG_CONFIG_DIRS under upstart user sessions). This will cause it to not start via XDG autostart exactly when user sessions are in use, which is probably what you really want. Having Unity in there smells a bit.

The problem is the opposite. We want it to start in non-Upstart user
sessions and use the Upstart job in Upstart user sessions.

Revision history for this message
Ted Gould (ted) :
review: Approve
Revision history for this message
Iain Lane (laney) wrote :

On Fri, Dec 06, 2013 at 10:15:35PM -0000, Ted Gould wrote:
> On Fri, 2013-12-06 at 17:44 +0000, Iain Lane wrote:
>
> > Instead of NotShowIn, you should ship a desktop file with Hidden=true in /usr/share/upstart/xdg/autostart/ (this directory is prepended to XDG_CONFIG_DIRS under upstart user sessions). This will cause it to not start via XDG autostart exactly when user sessions are in use, which is probably what you really want. Having Unity in there smells a bit.
>
>
> The problem is the opposite. We want it to start in non-Upstart user
> sessions and use the Upstart job in Upstart user sessions.

That's what I said.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Revision history for this message
Iain Lane (laney) wrote :

On Fri, Dec 06, 2013 at 10:21:17PM +0000, Iain Lane wrote:
> On Fri, Dec 06, 2013 at 10:15:35PM -0000, Ted Gould wrote:
> > On Fri, 2013-12-06 at 17:44 +0000, Iain Lane wrote:
> >
> > > Instead of NotShowIn, you should ship a desktop file with Hidden=true in /usr/share/upstart/xdg/autostart/ (this directory is prepended to XDG_CONFIG_DIRS under upstart user sessions). This will cause it to not start via XDG autostart exactly when user sessions are in use, which is probably what you really want. Having Unity in there smells a bit.
> >
> >
> > The problem is the opposite. We want it to start in non-Upstart user
> > sessions and use the Upstart job in Upstart user sessions.
>
> That's what I said.

To be clear, you ship both desktop files. See gnome-settings-daemon for
an example.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Revision history for this message
Ted Gould (ted) wrote :

On Fri, 2013-12-06 at 22:24 +0000, Iain Lane wrote:

> On Fri, Dec 06, 2013 at 10:21:17PM +0000, Iain Lane wrote:
> > That's what I said.
>
> To be clear, you ship both desktop files. See gnome-settings-daemon for
> an example.

Ah, I see. I was not aware of the override directory in Upstart. Yes,
we should do that for all the indicators. It is better than NoShowIn.

lp://staging/~charlesk/indicator-bluetooth/upstart-job updated
78. By Charles Kerr

add a second .desktop file to be installed in /usr/share/upstart/xdg/autostart with Hidden=true

79. By Charles Kerr

remove NotShowIn=Unity from the main .desktop file

80. By Charles Kerr

sync with trunk

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:80
http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-ci/29/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-trusty-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-trusty-armhf-ci/6
        deb: http://jenkins.qa.ubuntu.com/job/indicator-bluetooth-trusty-armhf-ci/6/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-bluetooth-ci/29/rebuild

review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

The desktop files look right to me, thanks!

(This is only a visual review)

review: Approve (desktop-files)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches