GNOME Bugzilla – Bug 790037
Make telepathy dependency optional
Last modified: 2018-01-01 03:07:19 UTC
See attached patch.
Created attachment 363179 [details] [review] Make telepathy dependency optional Telepathy is on the way out; remove the mandatory dependency from gnome-contacts.
Review of attachment 363179 [details] [review]: Thanks a lot for the patch! :-) Wholeheartedly agree with making Telepathy optional. Now, I did some cleaning up in the last few days and there are some things that changed or got removed, so would you mind doing some edits to the patch? ::: configure @@ +16,3 @@ DIRECTORY_OPTIONS='--@(prefix|bindir|sbindir|libexecdir|datadir|sysconfdir|libdir|mandir|includedir)' # Keep this in sync with meson_options.txt +PROJECT_OPTIONS='--@(with-cheese|with-telepathy|with-manpage)' I know this is not the case yet for the other options, but could you drop the "with-" prefix? (I want to change it at an exact moment, so Continuous doesn't break) ::: meson_options.txt @@ +1,2 @@ option('with-cheese', type: 'combo', choices: ['yes', 'no', 'auto'], value: 'auto', description: 'enable cheese webcam support') +option('with-telepathy', type: 'combo', choices: ['yes', 'no', 'auto'], value: 'auto', description: 'enable Telepathy call/chat support') I'm hesitant about keeping the "auto" situation in Cheese actually. In this case I'd say we drop it and set the default value on 'yes' (for now). Also the same remark as in configure. ::: src/contacts-contact.vala @@ +24,3 @@ +#if HAVE_TELEPATHY +using TelepathyGLib; +#endif It's better to just remove this statement and to use the namespace in the code. @@ +704,3 @@ +#else + if (p.store.can_remove_personas == MaybeBool.TRUE) { + return true; Please only use spaces for indentation. @@ +721,3 @@ +#else + if (p.store.can_remove_personas == MaybeBool.TRUE) { + personas.add (p); Please only use spaces for indentation. ::: src/contacts-store.vala @@ +24,2 @@ using TelepathyGLib; +#endif It's better to just remove this statement and to use the namespace in the code. @@ +329,3 @@ // TODO: listen for changes in Account#URISchemes private async void check_call_capabilities () { +#if HAVE_TELEPATHY Maybe include the whole method (and when it's called a few lines above separately) in the #if-directive. That way, someone can immediately see that it's Telepathy-specific @@ +347,3 @@ } +#if HAVE_TELEPATHY Merge this with the previous #if-directive.
Created attachment 366120 [details] [review] Make telepathy dependency optional -- Thanks for the review, Niels. Here's an updated and rebased patch that should address your comments.
Review of attachment 366120 [details] [review]: Looks good. Thanks! :-)
Attachment 366120 [details] pushed as edb3283 - Make telepathy dependency optional -- Thanks, pushed to master.