zeitgeist team mailing list archive

[Mikkel Kamstrup Erlandsen <mikkel.kamstrup@xxxxxxxxx>]

Review: Needs Fixing
 review needsfixing

> === modified file 'test/remote-test.py'
> +       def _create_mainloop(self):
> +               mainloop = gobject.MainLoop()
> +
> +               def cb_timeout():
> +                       mainloop.quit()
> +                       self.fail("Timed out -- operations not completed in reasonable time.")
> +
> +               # Add an arbitrary timeout so this test won't block if it fails
> +               gobject.timeout_add_seconds(30, cb_timeout)

I think 5s ought to be more than enough here? No-one will ever wait
30s for the timeout before they kill the proces anyway :-)

> === modified file 'zeitgeist/client.py'
> +       def set_data_source_enabled_callback(self, unique_id, enabled_callback):
> +               """
> +               This method may only be used after having registered the given unique_id
> +               with register_data_source before.
> +
> +               It registers a method to be called whenever the `enabled' status of
> +               the previously registered data-source changes.
> +
> +               Remember that on some systems the DataSourceRegistry extension may be
> +               disabled, in which case this method will have no effect.
> +               """
> +
> +               assert unique_id in self._data_sources, \
> +                       'set_data_source_enabled_callback() called before ' \
> +                       'register_data_source()'
> +
> +               assert callable(enabled_callback), \
> +                       'enabled_callback: expected a callable method'
> +
> +               self._data_sources[unique_id]['callback'] = enabled_callback

I don't think it's nice to assert in a library. You should throw
ValueError or TypeErrors instead I think.

-- 
https://code.launchpad.net/~zeitgeist/zeitgeist/bug695363/+merge/48950
Your team Zeitgeist Framework Team is subscribed to branch lp:~zeitgeist/zeitgeist/bug695363.