Re: Extra KDE Telepathy modules moving to Extragear
On Sat, Apr 28, 2012 at 22:44, Kevin Krammer wrote: > On Saturday, 2012-04-28, George Kiagiadakis wrote: > > > No, the classes that wrap GObjects do not need a d-pointer. All the > > calls are forwarded to the underlying GObject and if for any reason we > > ever need to save extra data on the wrapper class (which is highly > > unlikely), we should use the GObject qdata for that. Wrapper classes > > may be destroyed and re-allocated by QtGLib and therefore they > > shouldn't hold any data. > > So the GObject has a d-pointer? > > Any specific reason there is a GObject at all? My very basic understanding > of > Telepathy was that it is D-Bus based services. > Telepathy is based on D-Bus services, however this is about Telepathy logger [1], which is a GObject-based implementation of a central logging Telepathy component, which does not use D-Bus for sending the logs as it is quite heavy data and D-Bus for this purpose is rather slow, so it provides a direct access API. Our telepathy-logger-qt, which we want to move to Extragear, basically just wraps the original logger into Qt-like API, so that it can be used with Qt/KDE apps easily. [1] - http://telepathy.freedesktop.org/wiki/Logger -- Martin Klapetek | KDE Developer
Re: Review Request: udev PortableMediaPlayer: read protocols from media-player-info
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103028/#review13040
---
This review has been submitted with commit
3c1c788cc52e166278af2ed878e8a2f65874b1ac by Matěj Laitl to branch KDE/4.8.
- Commit Hook
On Dec. 3, 2011, 11:59 p.m., Matěj Laitl wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103028/
> ---
>
> (Updated Dec. 3, 2011, 11:59 p.m.)
>
>
> Review request for kdelibs.
>
>
> Description
> ---
>
> udev PortableMediaPlayer: read protocols from media-player-info
>
> This is a second attempt at implementing PortableMediaPlayer for udev
> back-end using media-player-info [3], the first attempt was [2] by
> Alex Merry and this patch is heavily based on it. This patch relates to
> a discussion at [1] and is just a first step, the second would
> be to forward PMP interface from udev backed to udisks backed somehow
> (udisks...Device interface provides NativePath attribute that links to
> sysfs path that can help - on Linux)
>
> [1] http://mail.kde.org/pipermail/kde-hardware-devel/2011-October/001481.html
> [2] https://svn.reviewboard.kde.org/r/5853/
> [3] http://www.freedesktop.org/wiki/Software/media-player-info
>
> Care is taken not to change existing behaviour - e.g. when udev env
> ID_MEDIA_PLAYER equals 1, behaviour is unchanged.
>
> PACKAGERS, solid udev backend now has following optional runtime-only
> dependency that provides udev rules and other info for identification
> of the portable media players:
> * media-player-info: for identifying USB storage devices and iPods
>
> Following packages also provide relevant udev rules, but we suggest not
> depending on them as they should by pulled by packages that actually
> use them (such as Amarok, transitively):
> * usbmuxd: for identifying iOS-based iPods
> * libmtp >= 1.0.4: for identifying MTP players
>
> CCBUG: 253671 # does not solve it yet, but is a first step
> CCBUG: 269447
> CCBUG: 269451
> REVIEW: 103028
> DIGEST: groundwork for better media device player detection
> CCMAIL: amarok-de...@kde.org
> CCMAIL: kde-packa...@kde.org
>
>
> Diffs
> -
>
> solid/solid/CMakeLists.txt 1a4adfad3b0aef700176e236f7587d3f26c76362
> solid/solid/backends/udev/udevdevice.cpp
> d6c7fb43427e7db4cb7cfa7d67a02c5368a7811e
> solid/solid/backends/udev/udevmanager.cpp
> 55e655b9f49875923d82b7f2bf10b0e23c3bdfe1
> solid/solid/backends/udev/udevportablemediaplayer.h
> e0348aafea7ec41e0df578dc661fbbb521531adf
> solid/solid/backends/udev/udevportablemediaplayer.cpp
> 32a189315bbc3bd323ef39f9e2743cbfe1063e68
> solid/solid/ifaces/portablemediaplayer.h
> b03a995223f03fa15c724427f05afdff6c3e7379
> solid/solid/xdgbasedirs.cpp PRE-CREATION
> solid/solid/xdgbasedirs_p.h PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/103028/diff/
>
>
> Testing
> ---
>
> 1. connect iPod
> 2. works:
> $ solid-hardware details
> /org/kde/solid/udev/sys/devices/pci:00/:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc
> udi =
> '/org/kde/solid/udev/sys/devices/pci:00/:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/host6/target6:0:0/6:0:0:0/block/sdc'
> parent = '/org/kde/solid/udev' (string)
> vendor = 'Apple' (string)
> product = 'iPod' (string)
> description = 'Portable Media Player' (string)
> Block.major = 8 (0x8) (int)
> Block.minor = 32 (0x20) (int)
> Block.device = '/dev/sdc' (string)
> PortableMediaPlayer.supportedProtocols = {'storage', 'ipod'} (string list)
> PortableMediaPlayer.supportedDrivers = {'usb'} (string list)
>
> 3. not yet:
> $ solid-hardware details /org/freedesktop/UDisks/devices/sdc1
> udi = '/org/freedesktop/UDisks/devices/sdc1'
> parent = '/org/freedesktop/UDisks/devices/sdc' (string)
> vendor = 'Apple' (string)
> product = 'MATOUSUV IP' (string)
> description = 'MATOUSUV IP' (string)
> Block.major = 8 (0x8) (int)
> Block.minor = 33 (0x21) (int)
> Block.device = '/dev/sdc1' (string)
> StorageAccess.accessible = true (bool)
> StorageAccess.filePath = '/media/MATOUSUV IP' (string)
> StorageAccess.ignored = false (bool)
> StorageVolume.ignored = false (bool)
> StorageVolume.usage = 'FileSystem' (0x2) (enum)
> StorageVolume.fsType = 'vfat' (string)
> StorageVolume.label = 'MATOUSUV IP' (string)
> StorageVolume.uuid = '3141-5926' (string)
> StorageVolume.size = 7888957440 (0x1d637f000) (qulonglong)
>
>
> Thanks,
>
> Matěj Laitl
>
>
Re: Extra KDE Telepathy modules moving to Extragear
On Saturday, 2012-04-28, George Kiagiadakis wrote: > No, the classes that wrap GObjects do not need a d-pointer. All the > calls are forwarded to the underlying GObject and if for any reason we > ever need to save extra data on the wrapper class (which is highly > unlikely), we should use the GObject qdata for that. Wrapper classes > may be destroyed and re-allocated by QtGLib and therefore they > shouldn't hold any data. So the GObject has a d-pointer? Any specific reason there is a GObject at all? My very basic understanding of Telepathy was that it is D-Bus based services. Cheers, Kevin -- Kevin Krammer, KDE developer, xdg-utils developer KDE user support, developer mentoring signature.asc Description: This is a digitally signed message part.
Re: Review request: AppMenu support for KDE
El Divendres, 27 d'abril de 2012, a les 09:35:14, Lionel Chauvin va escriure: > The code that bring support of AppMenu to KDE needs to be reviewed before it > entered in KDE main module: > https://projects.kde.org/projects/kdereview/kded-appmenu/repository > > It contains a KDED module and a library. > The KDED module exports applications menu through dbus. > The library exposes the functionalities of the module so it is not needed to > deal with KDED stuff. > > This support is required by the menu button in the oxygen decoration: > https://git.reviewboard.kde.org/r/104344/ > > It can be tested using an adapted version of the plasmoid menubar: > https://code.launchpad.net/~megabigbug/plasma-widget-menubar/appmenu-kde Can you clarify the license of the code? The COPYING says GPL3, kappmenuimporter.* seems to be not exact about the licensing, menuinfo.h says GPL3 again. Note that our licensing policy[1] does not accept GPL3-only code Do we really need generated files in the repository? (importer_interface.*) You have duplicate files $ fdupes -R . ./lib/com.canonical.AppMenu.Registrar.xml ./module/com.canonical.AppMenu.Registrar.xml Your installed library headers do not follow the of suggestions of our library policy, like no inline code in the headers, no d-pointers, etc. Cheers, Albert [1] http://techbase.kde.org/Policies/Licensing_Policy
Re: Extra KDE Telepathy modules moving to Extragear
El Dissabte, 28 d'abril de 2012, a les 16:08:41, George Kiagiadakis va escriure: > On Thu, Apr 26, 2012 at 11:07 PM, Albert Astals Cid wrote: > > El Dimecres, 25 d'abril de 2012, a les 18:15:00, David Edmundson va escriure: > >> We would like to move 2 more KDE Telepathy modules to Extragear, to > >> join our existing code. > >> > >> KTp Call UI [1] > >> > >> Provides a GUI for making video calls in telepathy. For details see > >> [2][3] > > > > Could you add some context to "Answer"? To the translator it's not obvious > > if it's the name or the verb that he is translating. > > Done. > > >> Telepathy Logger Qt [4] > >> > >> This provides Qt bindings for Telepathy-Logger a daemon that logs all > >> text messages received. This is an optional dependency for ktp-text-ui > >> which allows us to show a backlog and a way to view old log messages. > >> This was imported from Collabora's git repository. I moved it into KDE > >> playground where I'm maintaining it after Collabora seemed to lose any > >> interest in keeping it up to date or making a release. This has been > >> discussed on their mailing list. [5]. > > > > utils.h is installed but its class not exported? What's the reason for > > that? > Looks like a mistake. I'll fix it. > > > Some installed headers do not have a dpointer, i know this is a "obscure" > > library, but i think you should at least try to d-pointify them if you are > > making this a public library. > > No, the classes that wrap GObjects do not need a d-pointer. All the > calls are forwarded to the underlying GObject and if for any reason we > ever need to save extra data on the wrapper class (which is highly > unlikely), we should use the GObject qdata for that. Wrapper classes > may be destroyed and re-allocated by QtGLib and therefore they > shouldn't hold any data. What about the SearchHit struct? On a second header inspection i've also found a weird \ in pending-search.h Cheers, Albert
Re: Extra KDE Telepathy modules moving to Extragear
On Thu, Apr 26, 2012 at 11:07 PM, Albert Astals Cid wrote: > El Dimecres, 25 d'abril de 2012, a les 18:15:00, David Edmundson va escriure: >> We would like to move 2 more KDE Telepathy modules to Extragear, to >> join our existing code. >> >> KTp Call UI [1] >> >> Provides a GUI for making video calls in telepathy. For details see [2][3] > > Could you add some context to "Answer"? To the translator it's not obvious if > it's the name or the verb that he is translating. Done. >> >> Telepathy Logger Qt [4] >> >> This provides Qt bindings for Telepathy-Logger a daemon that logs all >> text messages received. This is an optional dependency for ktp-text-ui >> which allows us to show a backlog and a way to view old log messages. >> This was imported from Collabora's git repository. I moved it into KDE >> playground where I'm maintaining it after Collabora seemed to lose any >> interest in keeping it up to date or making a release. This has been >> discussed on their mailing list. [5]. > > utils.h is installed but its class not exported? What's the reason for that? Looks like a mistake. I'll fix it. > Some installed headers do not have a dpointer, i know this is a "obscure" > library, but i think you should at least try to d-pointify them if you are > making this a public library. No, the classes that wrap GObjects do not need a d-pointer. All the calls are forwarded to the underlying GObject and if for any reason we ever need to save extra data on the wrapper class (which is highly unlikely), we should use the GObject qdata for that. Wrapper classes may be destroyed and re-allocated by QtGLib and therefore they shouldn't hold any data.
Re: Review Request: New KDE Macro for to wrap the noreturn attribute
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103832/#review13016 --- it's not necessary to add that _feature_ to 4.8, and for frameworks qt5 has an equivalent macro. - Oswald Buddenhagen On Jan. 31, 2012, 8:58 p.m., Allen Winter wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103832/ > --- > > (Updated Jan. 31, 2012, 8:58 p.m.) > > > Review request for kdelibs. > > > Description > --- > > This diff adds a new macro KDE_NO_RETURN that wraps the noreturn attribute > which is enabled differently based on the compiler. > > > Diffs > - > > kdemacros.h.cmake b93242c > > Diff: http://git.reviewboard.kde.org/r/103832/diff/ > > > Testing > --- > > did a test compile > > > Thanks, > > Allen Winter > >
