New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change settings to look at a more relevant value for time format #222

Merged
merged 1 commit into from Oct 3, 2018

Conversation

Projects
None yet
6 participants
@dahenson
Copy link
Contributor

dahenson commented Oct 3, 2018

This particular PR will fix elementary/calendar#268 and fix elementary/calendar#91.

This will only fix those issues for Calendar in elementary, but it will probably break things in other systems that don't have the wingpanel indicator, or a way to change the format of the wingpanel indicator.

Either way, it seems reasonable to expect Granite to respect the system setting for elementary over anything else.

@danrabbit danrabbit merged commit 558502d into elementary:master Oct 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hosiet

This comment has been minimized.

Copy link

hosiet commented Dec 20, 2018

I'm wondering if there could be any fallback option for systems on which io.elementary.desktop.wingpanel.datetime does not exist. Otherwise the library will crash in such circumstances.

Ref: https://bugs.debian.org/916173

@tintou

This comment has been minimized.

Copy link
Member

tintou commented Jan 4, 2019

This shouldn't have been done, the clock-format from the GNOME scheme is the only one that we should use and is enough

@dahenson

This comment has been minimized.

Copy link
Contributor

dahenson commented Jan 4, 2019

This may have been done a little to hastily, but if the GNOME scheme is the only one that should be used, it should be used everywhere, which is not the case for calendar I believe (among other things).

@JoshStrobl

This comment has been minimized.

Copy link

JoshStrobl commented Jan 11, 2019

This change was made with seemingly zero consideration for any other platforms or application developers which may leverage Granite but still wish to ship their apps on other non-elementary OS platforms, as a key example being bleakgrey/tootle#107. The org.gnome.desktop.datetime schema can reliably be found on other platforms where GTK applications and gsettings-desktop-schemas (which is what ships the schema) are, such as:

  • Platforms / operating systems which ship Budgie Desktop, which uses gsettings-desktop-schemas as well as gnome-session which depends on it.
  • Platforms / operating systems which ship GNOME Shell, which uses gsettings-desktop-schemas as well as gnome-session (which GNOME Shell and GNOME Session Shell depend on).
  • Platforms / operating systems which ship MATE and leverage mate-session-manager
  • Platforms / operating systems which ship KDE and leverage kde-gtk-config to apply GTK settings.

tl;dr - Literally everywhere else. I've personally reverted this change downstream in Solus.

@EbonJaeger

This comment has been minimized.

Copy link

EbonJaeger commented Jan 11, 2019

"which is not the case for calendar I believe (among other things)."

So maybe fix those instead of maybe breaking stuff for everyone else?

@JoshStrobl

This comment has been minimized.

Copy link

JoshStrobl commented Jan 11, 2019

PR submitted in #256

@dahenson

This comment has been minimized.

Copy link
Contributor

dahenson commented Jan 11, 2019

"which is not the case for calendar I believe (among other things)."

So maybe fix those instead of maybe breaking stuff for everyone else?

You are absolutely right. This was done in error, and should be reverted. It was not my intent to break things for everyone else.

@JoshStrobl

This comment has been minimized.

Copy link

JoshStrobl commented Jan 11, 2019

@dahenson I submitted a PR which maintains your change while making sure it works with the prior schema. If you'd like to test it (#256) it'd be appreciated 🙂 Alternatively change can be reverted entirely and thus make my PR obsolete. Either I'm fine with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment