Review: Needs Fixing
Hey,
Good job, thanks for working on this.
Some comments:
> (this.network != null) { ... }
What's this supposed to do? You've already unwatched everything in the "else {
... }", so you can just return.
> this.network.setup ();
I'm not too happy about this. Hm. Let's leave it like this, but at least add a
comment to the abstract setup() document that it will call
on_network_up/on_network_up with the initial state of the network.
> public interface NetworkMonitor: Object
Shouldn't this be private? It's not supposed to be used anywhere outside the
extension.
> Write connectivity to the DB.
Not sure what this comment is supposed to mean.
> private void name_appeared_handler(DBusConnection connection, string name,
> string name_owner)
You're missing a space between "handler (".
> Checks whether there is a funtioning
Typo (funtioning -> functioning). I'd reword it to something like "Monitor the
availability of working network connections using Network Manager (0.8 or
later)".
> // ^^ There is a bug in some Connman versions causing it to not emit the
This comment is about the proxy.state_changed.connect, but the position where
it's places make it look like it's proxy.get_state.
--
https://code.launchpad.net/~cando/zeitgeist/storage-monitor/+merge/84299
Your team Zeitgeist Framework Team is subscribed to branch lp:zeitgeist.
_______________________________________________
Mailing list: https://launchpad.net/~zeitgeist
Post to : zeitgeist@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zeitgeist
More help : https://help.launchpad.net/ListHelp
