Merge lp://staging/~sinzui/bzr-gtk/notify-gtk-import into lp://staging/bzr-gtk

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: 776
Merge reported by: Curtis Hovey
Merged at revision: not available
Proposed branch: lp://staging/~sinzui/bzr-gtk/notify-gtk-import
Merge into: lp://staging/bzr-gtk
Diff against target: 232 lines (+99/-21)
5 files modified
bzr-notify (+24/-14)
notify.py (+9/-7)
preferences/plugins.py (+3/-0)
tests/__init__.py (+1/-0)
tests/test_notify.py (+62/-0)
To merge this branch: bzr merge lp://staging/~sinzui/bzr-gtk/notify-gtk-import
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) Approve
Review via email: mp+91497@code.staging.launchpad.net

Description of the change

Update bzr-notify to Gtk3 and AppIndicator3.

    Launchpad bug: https://bugs.launchpad.net/bugs/903444
    Pre-implementation: no one

I see a stacktrace that indicates that bzr-notify is importing gtk2 libs.
    $ bzr-notify
    Traceback (most recent call last):
      File "/usr/bin/bzr-notify", line 20, in <module>
        import gobject
      File "/usr/lib/python2.7/dist-packages/gobject/__init__.py", line 47...
        from gobject.constants import *
      File "/usr/lib/python2.7/dist-packages/gobject/constants.py", line 23...

--------------------------------------------------------------------

RULES

    * Update the imports to use gir.
    * Addendum
      * pynotify and appindicator needs updating to gir.
        The precise bzr-gtk package appears to recommend
        gir1.2-appindicator3-0.1 | gir1.2-notify-0.7
        but I do not think they were actually used.

QA

    Using Unity/appindicator3-0
    * Switch your .bazaar/plugins/gtk to point to this branch.
    * Run bzr-notify from the tree
    * Commit a change to a test branch
    * Verify the notification appears
    * Verify the indicator icon appears in the panel
    * Quickly open the bzr-gtk preferences from the indicator menu.
    * When the indicator disappears, close the preferences window
    * Verify there is not an error in the about a bad path:
      p = bzrlib.plugin.plugins()[self.model[path][0]].module

    I tested Notify and instrumenting an import failure of AppIndicator3.
    The notification appeared, but my I did not see the notification icon
    appear. There were no errors in the terminal. I added bzr-notify to
    the dconf notification area exceptions, but that still did not let
    the icon appear.

IMPLEMENTATION

Fixed a race condition where a cursor change event is fired while the
widget is being destroyed. I discovered this while watching the console
when I closed the preferences window.
  preferences/plugins.py

Updated the modules to import from GObject, Notify, and AppIndicator3.
The test for bzr-notify is crude. It will report of the process fails to
start because of import or system errors, but cannot test the actual
function. We need to move the two menu implementations and the message
implementation to another module to test them.
  bzr-notify
  notify.py
  tests/__init__.py
  tests/test_notify.py

To post a comment you must log in.
776. By Curtis Hovey

Use the one-true way to setup the plugin path for the test.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Nice, and great to have some smoke tests.

review: Approve

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

to all changes: