Created attachment 208848 [details] [review]
Add org.gnome.login-screen banner-message setting

Created attachment 208849 [details] [review]
Show configured banner message

We used banner_message_text to distinguish between central managed systems and user managed/installed systems. Patches added

Review of attachment 208849 [details] [review]:

hey thanks for working on this!

::: js/gdm/loginDialog.js
@@ +787,3 @@
+                                           text: ''});
+        this.contentLayout.add(this._bannerLabel);
+        this._updateBanner();

Don't you also need to call this on the "changed::" + _BANNER_TEXT_KEY signal of the _settings object ?

Created attachment 208865 [details] [review]
v2 of the patch

Definitely. Added bind to change:: and renamed _BANNER_TEXT_KEY to _BANNER_MESSAGE_KEY to closer reflect banner-message.

Review of attachment 208865 [details] [review]:

Hey sorry for the really slow turn around on this patch. Thank you for the time you put into this.

::: data/theme/gdm.css
@@ +24,3 @@
+    padding-bottom: 1em;
+}
+

We should probably get designer feedback here. But seems reasonable at a glance.

::: js/gdm/loginDialog.js
@@ +50,3 @@
 const _LOGIN_SCREEN_SCHEMA = 'org.gnome.login-screen';
 const _FINGERPRINT_AUTHENTICATION_KEY = 'enable-fingerprint-authentication';
+const _BANNER_MESSAGE_KEY = 'banner-message';

There are two keys:

    <key name="banner-message-enable" type="b">
      <default>false</default>
      <_summary>
        Enable showing the banner message
      </_summary>
      <_description>
        Set to true to show the banner message text.
      </_description>
    </key>
    <key name="banner-message-text" type="s">
      <default>''</default>
      <_summary>
        Banner message text
      </_summary>
      <_description>
        Text banner message to show in the login window.
      </_description>
    </key>

so __BANNER_MESSAGE_KEY needs to be 'banner-message-text' and also need to look at 'banner-message-enable'

Review of attachment 208848 [details] [review]:

( obsolete - marking rejected to get off of review list )

Created attachment 218142 [details] [review]
Implemented banner support for the login screen

Based on a patch by Marius Rieder,
https://bugzilla.gnome.org/review?bug=665346

Lets not let this linger forever... here is a new patch, using the correct keys. Tested to work.

Review of attachment 218142 [details] [review]:

Seems fine to me, sans one minor nit.

::: js/gdm/loginDialog.js
@@ +793,3 @@
 
+        this._bannerLabel = new St.Label({ style_class: 'login-dialog-banner',
+                                           text: ''});

Missing whitespace here.

I've pushed it with the space I think you were missing. Your review comments are sometimes a bit too terse to be entirely sure...