Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/storagemonitor2 into lp:zeitgeist
Review: Approve Alright, I've tested it with Network Manager and it seems to work fine. Awesome work! Since I love nitpicking so much, "self._up ()" could be changed to "self._up()" :P. But anyway, I think this is ready to merge as soon as dbschema4 goes in (outstanding issues for that are deciding what to do with 0.7 compatibility, merging Seif's move tracking and deciding if we like Markus' cache deletion workaround - am I forgetting anything?). -- https://code.launchpad.net/~zeitgeist/zeitgeist/storagemonitor2/+merge/49212 Your team Zeitgeist Framework Team is subscribed to branch lp:~zeitgeist/zeitgeist/storagemonitor2. ___ Mailing list: https://launchpad.net/~zeitgeist Post to : zeitgeist@lists.launchpad.net Unsubscribe : https://launchpad.net/~zeitgeist More help : https://help.launchpad.net/ListHelp
Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/storagemonitor2 into lp:zeitgeist
Review: Needs Fixing
[2011-03-04 22:48:11,119] - ERROR - zeitgeist.extension - Failed loading the
'StorageMonitor' extension
Traceback (most recent call last):
File
"/home/rainct/Desenvolupament/Python/zeitgeist-project/storagemonitor2/zeitgeist/../_zeitgeist/engine/extension.py",
line 265, in load
obj = extension(self.__engine)
File
"/home/rainct/Desenvolupament/Python/zeitgeist-project/storagemonitor2/zeitgeist/../_zeitgeist/engine/extensions/storagemonitor.py",
line 125, in __init__
lambda: self.remove_storage_medium("net"))
File
"/home/rainct/Desenvolupament/Python/zeitgeist-project/storagemonitor2/zeitgeist/../_zeitgeist/engine/extensions/storagemonitor.py",
line 326, in __init__
proxy = dbus.SystemBus().get_object(NetworkMonitor.NM_BUS_NAME,
NameError: global name 'NetworkMonitor' is not defined
--
https://code.launchpad.net/~zeitgeist/zeitgeist/storagemonitor2/+merge/49212
Your team Zeitgeist Framework Team is subscribed to branch
lp:~zeitgeist/zeitgeist/storagemonitor2.
___
Mailing list: https://launchpad.net/~zeitgeist
Post to : zeitgeist@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zeitgeist
More help : https://help.launchpad.net/ListHelp
Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/storagemonitor2 into lp:zeitgeist
Haven't tried it yet, but the code looks good to me (although I still think a baseclass would be neat :P). -- https://code.launchpad.net/~zeitgeist/zeitgeist/storagemonitor2/+merge/49212 Your team Zeitgeist Framework Team is subscribed to branch lp:~zeitgeist/zeitgeist/storagemonitor2. ___ Mailing list: https://launchpad.net/~zeitgeist Post to : zeitgeist@lists.launchpad.net Unsubscribe : https://launchpad.net/~zeitgeist More help : https://help.launchpad.net/ListHelp
Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/storagemonitor2 into lp:zeitgeist
> > "lambda : " > The space before the colons looks ugly :P. Fixed > > "except:" > Please make this "except sqlite3.foobar" (or, worst-case, "except Exception"). > Also, why do you need the rollback there? Since sqlite3 doesn't have a common error super class I am just catching Exception for now. The rollback() has been changed to 'return' instead. > > "A storage medium is indetified by a key" > How can I indetify you? :) Fixed > You could move most the the NM/Connman code into a common base class. Yeah, I was thinking that, but there is so little code in them that I think a common base class would almost give us more net lines :-) -- https://code.launchpad.net/~zeitgeist/zeitgeist/storagemonitor2/+merge/49212 Your team Zeitgeist Framework Team is subscribed to branch lp:~zeitgeist/zeitgeist/storagemonitor2. ___ Mailing list: https://launchpad.net/~zeitgeist Post to : zeitgeist@lists.launchpad.net Unsubscribe : https://launchpad.net/~zeitgeist More help : https://help.launchpad.net/ListHelp
Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/storagemonitor2 into lp:zeitgeist
Review: Needs Fixing > "lambda : " The space before the colons looks ugly :P. > "except:" Please make this "except sqlite3.foobar" (or, worst-case, "except Exception"). Also, why do you need the rollback there? > "A storage medium is indetified by a key" How can I indetify you? :) You could move most the the NM/Connman code into a common base class. Looks great otherwise. Good job!!! -- https://code.launchpad.net/~zeitgeist/zeitgeist/storagemonitor2/+merge/49212 Your team Zeitgeist Framework Team is subscribed to branch lp:~zeitgeist/zeitgeist/storagemonitor2. ___ Mailing list: https://launchpad.net/~zeitgeist Post to : zeitgeist@lists.launchpad.net Unsubscribe : https://launchpad.net/~zeitgeist More help : https://help.launchpad.net/ListHelp
Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/storagemonitor2 into lp:zeitgeist
Review: Needs Fixing +logging.basicConfig(level=logging.DEBUG) WHY? And why is this and the imports there twice (before and after the big comments)? +# storgaemonitor extension. This is actually backwards compatible. Storgae? Is that like 'deine mudda' in Danish? -- https://code.launchpad.net/~zeitgeist/zeitgeist/storagemonitor2/+merge/49212 Your team Zeitgeist Framework Team is subscribed to branch lp:~zeitgeist/zeitgeist/storagemonitor2. ___ Mailing list: https://launchpad.net/~zeitgeist Post to : zeitgeist@lists.launchpad.net Unsubscribe : https://launchpad.net/~zeitgeist More help : https://help.launchpad.net/ListHelp
