← Back to team overview

zeitgeist team mailing list archive

Re: [Merge] lp:~zeitgeist/zeitgeist/fts++ into lp:zeitgeist

 

> Awesome! C++ FTS ftw.
> 
> - Add COPYING.GPL3, otherwise the tarball can't be re-distributed.
> 

On it...

> - Considering sharing a get_flags_for_log_level or even set_log_level
>   function between ZG and FTS?
> 

I don't think that's really necessary, strictly speaking it'd be a utility function for a specific app, and has no place in a library.

> - s/ver != DatabaseSchema.CORE_SCHEMA_VERSION)/ver <
> DatabaseSchema.CORE_SCHEMA_VERSION/
>   What's the rationale for this? We don't know changes won't break
> compatibility
> 

Does that mean we should automatically assume that the possible changes do break stuff? This is only used with read-only database so I don't see any harm - either the reading will continue to work or you'll get some run-time errors, I find that better than just not working with even trying.

> - Can you explain the "// Don't disconnect monitors using service names"?
> 

As said on IRC, it prevents some races by allowing the internal extensions to register a monitor with a service name (races that would otherwise cause missed notifications when the external daemon is starting and didn't have a chance to register a monitor)

> I didn't really review the C++ stuff (I'm asuming you and Mikkel reviewed each
> other's stuff already?).

Partially, but we have tests, so it has to work, right?! :)
-- 
https://code.launchpad.net/~zeitgeist/zeitgeist/fts++/+merge/92022
Your team Zeitgeist Framework Team is subscribed to branch lp:zeitgeist.


References