[Merge] lp:~phablet-team/messaging-framework/destroyable_channels into lp:messaging-framework

Roberto Mier Escandón  roberto.escandon at canonical.com
Wed Jun 8 11:38:00 UTC 2016


Review: Needs Information

Nice! I thought this kind of Telepathy implementations would only be included in TelepathyQt project, but looks nice this way. I'm thinking about a bunch of other implementations that we would need in future.
Just a comment below, and a question here:
Shouldn't this interface belong to a namespace, let's say, "messaging::tp::qt" or directly "Tp"?

Maybe in future we can shorten them by removing obvious ones


Diff comments:

> 
> === modified file 'src/CMakeLists.txt'
> --- src/CMakeLists.txt	2016-05-30 16:43:03 +0000
> +++ src/CMakeLists.txt	2016-06-07 23:31:48 +0000
> @@ -1,7 +1,10 @@
>  # Expose internal headers under sensible paths to the impl.
>  include_directories(${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/messaging ${PROCESS_CPP_INCLUDE_DIRS} ${LibPhoneNumber_INCLUDE_DIRS})
>  
> -qt5_wrap_cpp(MESSAGING_FW_MOCS ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/protocol.h ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/connection.h)
> +qt5_wrap_cpp(MESSAGING_FW_MOCS ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/protocol.h
> +                               ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/connection.h
> +                               ${CMAKE_SOURCE_DIR}/include/messaging/qt/tp/interfaces/base_channel_destroyable.h
> +                               ${CMAKE_SOURCE_DIR}/src/messaging/qt/tp/interfaces/base_channel_destroyable_internal.h)

This sounds weird to me having a .h in src, but I guess there is an explanation related with Qt moc....

If this must be done this way, maybe we should move the fragment

# make the .h files visible on qtcreator
file(GLOB_RECURSE H_FILES *.h)
add_custom_target(dot_h_files ALL SOURCES ${H_FILES})

from include/CMakeLists.txt to root CMakeLists.txt in order to QtCreator show .h files in src subfolder.

>  
>  # The general messaging-fw shared library.
>  add_library(


-- 
https://code.launchpad.net/~phablet-team/messaging-framework/destroyable_channels/+merge/296745
Your team Ubuntu Phablet Team is subscribed to branch lp:messaging-framework.



More information about the Ubuntu-reviews mailing list