Merge lp://staging/~laney/ubuntu-system-settings/lp1631997 into lp://staging/ubuntu-system-settings

Proposed by Iain Lane
Status: Needs review
Proposed branch: lp://staging/~laney/ubuntu-system-settings/lp1631997
Merge into: lp://staging/ubuntu-system-settings
Diff against target: 8 lines (+1/-0)
1 file modified
ubuntu-system-settings.desktop.in.in (+1/-0)
To merge this branch: bzr merge lp://staging/~laney/ubuntu-system-settings/lp1631997
Reviewer Review Type Date Requested Status
Ken VanDine Needs Fixing
Jonas G. Drange (community) Approve
Review via email: mp+308065@code.staging.launchpad.net

Commit message

Add OnlyShowIn=Unity8 to the desktop file, so we don't get two "System Settings" in a Unity 7 session, now that u-s-s is installed by default.

To post a comment you must log in.
Revision history for this message
Iain Lane (laney) wrote :

I think I would prefer to upload this as an ubuntu2, and not wait for it to be merged, and not push it myself.

1) There's not much time before 16.10 now and it would be good for this to be on images (QA might take a while)
2) I'm not sure whether this OnlyShowIn will work on the vivid/xenial phone environments so might not be ready to land there

Feel free to do whatever you think is best with this MP

Revision history for this message
Jonas G. Drange (jonas-drange) wrote :

I only know the citrain process, so I've created silo 2063.

Anyway, looks good.

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

On Tue, Oct 11, 2016 at 11:53:58AM -0000, Jonas G. Drange wrote:
> Review: Approve
>
> I only know the citrain process, so I've created silo 2063.
>
> Anyway, looks good.

Cheers. No rush to upload to yakkety though (actually, it would probably
be disruptive to do that now), so take your time to QA this properly. :)

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

Revision history for this message
Ken VanDine (ken-vandine) wrote :

Turns out OnlyShowIn isn't honored in the app scope in unity8, which results in system-settings being hidden in unity8.

This works in unity8 and does hide it in unity7:

NotShowIn=Unity

review: Needs Fixing
Revision history for this message
Ken VanDine (ken-vandine) wrote :

> Turns out OnlyShowIn isn't honored in the app scope in unity8, which results
> in system-settings being hidden in unity8.
>
> This works in unity8 and does hide it in unity7:
>
> NotShowIn=Unity

I think you can include gnome and kde as well, it's not ideal but unfortunately unity8 doesn't support OnlyShowIn :(

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

Is XDG_CURRENT_DESKTOP set? It certainly worked for us in yakkety's unity 8 desktop session where it is set to Unity:Unity8.

NotShowIn=Unity isn't a great idea, for your reason, because XDG_CURRENT_DESKTOP contains "Unity" already in U8 on desktop (as I just said) and because the moment that U8 supports Only/NotShowIn properly then things will stop showing up if XDG_CURRENT_DESKTOP contains "Unity" elsewhere too - and it would be weird for it to be different depending on how you get into Unity8.

I think that it's preferable for OnlyShowIn to be properly supported instead.

Revision history for this message
Ken VanDine (ken-vandine) wrote :

> Is XDG_CURRENT_DESKTOP set? It certainly worked for us in yakkety's unity 8
> desktop session where it is set to Unity:Unity8.
>
> NotShowIn=Unity isn't a great idea, for your reason, because
> XDG_CURRENT_DESKTOP contains "Unity" already in U8 on desktop (as I just said)
> and because the moment that U8 supports Only/NotShowIn properly then things
> will stop showing up if XDG_CURRENT_DESKTOP contains "Unity" elsewhere too -
> and it would be weird for it to be different depending on how you get into
> Unity8.
>
> I think that it's preferable for OnlyShowIn to be properly supported instead.

XDG_CURRENT_DESKTOP is not set on the device. Also dobey said the scope doesn't do anything to filter that out anyway, but I'm pretty sure the icon was showing before with this branch in the silo.

Unmerged revisions

1729. By Iain Lane

Add OnlyShowIn=Unity8 to the desktop file

So we don't get two "System Settings" in a Unity 7 session, now that u-s-s is
installed by default.

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