[Kde-hardware-devel] Review Request 117806: [WIP] Solid device import with proper model support

Aleix Pol Gonzalez aleixpol at kde.org
Sun Apr 27 22:09:42 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117806/#review56697
-----------------------------------------------------------


Also, have you run the ModelTest against it?


src/imports/declarativedevices.h
<https://git.reviewboard.kde.org/r/117806/#comment39550>

    I would remove this property. It adds quite some logic and complexity for no apparent reason. You can use count==0 instead.



src/imports/declarativedevices.cpp
<https://git.reviewboard.kde.org/r/117806/#comment39547>

    You probably want to do a begin/endRemoveRows or begin/endResetModel.



src/imports/declarativedevices.cpp
<https://git.reviewboard.kde.org/r/117806/#comment39548>

    You want to actually add it here, no?
    



src/imports/declarativedevices.cpp
<https://git.reviewboard.kde.org/r/117806/#comment39549>

    No need to remove anything?
    



src/solid/CMakeLists.txt
<https://git.reviewboard.kde.org/r/117806/#comment39551>

    Don't comment, remove, no?


- Aleix Pol Gonzalez


On April 27, 2014, 9:41 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117806/
> -----------------------------------------------------------
> 
> (Updated April 27, 2014, 9:41 p.m.)
> 
> 
> Review request for Solid, Àlex Fiestas, Aleix Pol Gonzalez, Ivan Čukić, and Lukáš Tinkl.
> 
> 
> Repository: solid
> 
> 
> Description
> -------
> 
> This adds proper model support to the Solid device import making it possible to hook it up to a ListView directly with automatically propagated change notifications.
> 
> You can have a look at a WIP battery monitor using it in [plasma-workspace] broulik/batterymonitorsolidimport branch.
> 
> I'm not completely satisfied with it as I'm not really good at that Qt model stuff, having the private impl mess around with the QList and having the model itself just calling beginInsertRows and endInsertRows is probably not optimal.
> 
> I apologize for the hard-to-read diff but I renamed the files outside of git and git diff --find-copies-harder confused Review Board :/ It is also available in the broulik/modelimport branch.
> 
> 
> Diffs
> -----
> 
>   src/imports/CMakeLists.txt 8c579ef 
>   src/imports/declarativedevices.h PRE-CREATION 
>   src/imports/declarativedevices.cpp PRE-CREATION 
>   src/imports/declarativedevices_p.h PRE-CREATION 
>   src/imports/devices.h e321c5c 
>   src/imports/devices.cpp a586728 
>   src/imports/devices_p.h 7d5a1f9 
>   src/imports/solidextensionplugin.cpp 6a959f1 
>   src/solid/CMakeLists.txt 8fb91da 
>   src/solid/backends/fakehw/fakebattery.h f544613 
>   src/solid/backends/fakehw/fakebattery.cpp 9af4c6c 
>   src/solid/backends/fakehw/fakedeviceinterface.h c704b44 
>   src/solid/backends/fakehw/fakedeviceinterface.cpp b5f5821 
>   src/solid/backends/fstab/fstabnetworkshare.h 775999c 
>   src/solid/backends/fstab/fstabnetworkshare.cpp 86f56e6 
>   src/solid/backends/fstab/fstabstorageaccess.h d608387 
>   src/solid/backends/fstab/fstabstorageaccess.cpp ca18502 
>   src/solid/backends/hal/halbattery.h 71d654d 
>   src/solid/backends/hal/halbattery.cpp a2ac83d 
>   src/solid/backends/hal/halblock.h f0310f1 
>   src/solid/backends/hal/halblock.cpp 1d3e51c 
>   src/solid/backends/hal/halcamera.h c7c339e 
>   src/solid/backends/hal/halcamera.cpp 562568d 
>   src/solid/backends/hal/halcdrom.cpp b35b0de 
>   src/solid/backends/hal/haldevice.cpp e878576 
>   src/solid/backends/hal/halgenericinterface.h 400bd10 
>   src/solid/backends/hal/halgenericinterface.cpp db7cd40 
>   src/solid/backends/hal/halopticaldisc.cpp b3004be 
>   src/solid/backends/hal/halportablemediaplayer.h e82e825 
>   src/solid/backends/hal/halportablemediaplayer.cpp fde65bd 
>   src/solid/backends/hal/halprocessor.h 4f75136 
>   src/solid/backends/hal/halprocessor.cpp 31959ae 
>   src/solid/backends/hal/halstorage.cpp 7ded650 
>   src/solid/backends/hal/halstorageaccess.h f8ad7d1 
>   src/solid/backends/hal/halstorageaccess.cpp 7463212 
>   src/solid/backends/hal/halvolume.cpp 4dd4bcc 
>   src/solid/backends/udev/udevblock.h 294b157 
>   src/solid/backends/udev/udevblock.cpp d7dc110 
>   src/solid/backends/udev/udevcamera.h 3b0b1a4 
>   src/solid/backends/udev/udevcamera.cpp fa89055 
>   src/solid/backends/udev/udevdeviceinterface.h 5820a6d 
>   src/solid/backends/udev/udevdeviceinterface.cpp b67592c 
>   src/solid/backends/udev/udevgenericinterface.h b7a0b2b 
>   src/solid/backends/udev/udevgenericinterface.cpp f513cec 
>   src/solid/backends/udev/udevportablemediaplayer.h 6cb416c 
>   src/solid/backends/udev/udevportablemediaplayer.cpp 907051c 
>   src/solid/backends/udev/udevprocessor.h f424aab 
>   src/solid/backends/udev/udevprocessor.cpp 80d14f5 
>   src/solid/backends/udisks2/udisksdeviceinterface.h 871aad9 
>   src/solid/backends/udisks2/udisksdeviceinterface.cpp 9fa60e5 
>   src/solid/backends/upower/upowerbattery.h f0f41b5 
>   src/solid/backends/upower/upowerbattery.cpp c771f68 
>   src/solid/backends/upower/upowerdeviceinterface.h 2fcb322 
>   src/solid/backends/upower/upowerdeviceinterface.cpp 0ee5674 
>   src/solid/battery.h bbf2201 
>   src/solid/battery.cpp 3cc376f 
>   src/solid/device.cpp 93d77e0 
>   src/solid/deviceinterface.h dd212cc 
>   src/solid/deviceinterface.cpp 7c7222f 
>   src/solid/ifaces/backenddeviceinterface.h PRE-CREATION 
>   src/solid/ifaces/backenddeviceinterface.cpp PRE-CREATION 
>   src/solid/ifaces/battery.h 864f93d 
>   src/solid/ifaces/device.h 893823d 
>   src/solid/ifaces/deviceinterface.h 5c14496 
>   src/solid/ifaces/deviceinterface.cpp 32ec160 
>   src/solid/managerbase.cpp 8eb97ad 
>   src/solid/udevdeviceinterface.h PRE-CREATION 
>   src/solid/udevdeviceinterface.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/117806/diff/
> 
> 
> Testing
> -------
> 
> I tried to iron out most of the crashes and issues and that thing will definitely need a whole lot of unit tests.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20140427/e2d42b7f/attachment-0001.html>


More information about the Kde-hardware-devel mailing list