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
Re: Extra KDE Telepathy modules moving to Extragear
On Thu, Apr 26, 2012 at 11:07 PM, Albert Astals Cid aa...@kde.org 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: 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 aa...@kde.org 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: 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
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: 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 Sat, Apr 28, 2012 at 22:44, Kevin Krammer kram...@kde.org 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
