On Fri, 2014-04-25 at 12:00 +0000, Potrola, MateuszX wrote: > What do you think about this proposal ?
Hi, thanks for it, but please open a bug report (and send a link to it here), where we'll do all the review work. It will take couple iterations. From a brief look into the patch: a) license blocks are not correct in the new files, please use those from the current git master (the cleanup took me several hours) b) the new "..._non_locking" functions, following GAsyncQueue API pattern, the suffix should better be "..._unlocked" c) add developer comments to the new "private" functions and define them in the header - it'll save you many headaches in the future d) there are some coding style issues, like (there are more similar) at the top of e-book-sqlite.h: - removed empty lines, which should stay there - the added #include "e-book-sqlite-changelog.h" should be done similar to the #include above it, thus it'll work even outside of eds, but: e) is the EBookSqliteChangelog needed at all? What I had on mind were added signals in the EBookSqlite, which would be called on places where you currently engage the EBookSqliteChangelog functions. That way, with the signals, you do not need to know anything about the caller, neither specifically list the changelog from the extensions list - it is currently odd too, because if there are two changelog extensions then the way you have it right now will engage only one, hard to tell which of them, though f) I would add the ESource argument also to e_book_sqlite_new() - as it will change API, you need to bump soname version for libedatabook. You need to bump it anyway, for the change of e_book_sqlite_new_full(). g) I do not like the changes in e_book_sqlite_class_init(), because: - there is a possible memory leak of modules_directory - load of extensions should be done transparently, supposing you save your extension to the proper modules' directory, which is $PREFIX/lib/evolution-data-server/addressbook-backends Hrm, pity it's named 'backends', but it belongs to the addressbook factory, thus it's it. What you should do in the EBookSqlite is to simply call e_extensible_load_extensions() in GObject::constructed override in EBookSqlite, after the parent struct 'constructed' is called. h) I did not get from the changes how you'll access the sqlite3 database for writing. All the added "private" functions are for reading - they seem to be to me at least. In any case, as I said above, let's continue in the bug report. Bye, Milan _______________________________________________ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers