Review Request 126851: Places data engine: Rename model role name "index" to "id"

Marco Martin notmart at gmail.com
Mon Jan 25 08:50:18 UTC 2016


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


Ship it!




on the fence about this.
it's a rename of a semi public thing that may cause problems, but if it breaks the qml model uhm, ok

- Marco Martin


On Jan. 23, 2016, 2 p.m., Daniel Faust wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126851/
> -----------------------------------------------------------
> 
> (Updated Jan. 23, 2016, 2 p.m.)
> 
> 
> Review request for kde-workspace and Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> I wrote a qml applet using the places data engine and noticed that I can't access the index variable of ListView items because it gets overwritten by the places model.
> This patch renames the "index" role name to "id" in order to avoid this naming conflict.
> 
> 
> Diffs
> -----
> 
>   dataengines/places/org.kde.places.operations 98a951d 
>   dataengines/places/placeservice.cpp e0c93c5 
>   dataengines/places/placesproxymodel.h 467fe83 
>   dataengines/places/placesproxymodel.cpp 5ea807b 
> 
> Diff: https://git.reviewboard.kde.org/r/126851/diff/
> 
> 
> Testing
> -------
> 
> I couldn't find any other model that is using the places data engine so I hope renaming the model role is fine.
> The provided model still works and I tested the "Setup Device" and "Teardown Device" operations of the service.
> The operations "Show" and "Hide" don't seem to work anyway and the others were not tested.
> 
> 
> Thanks,
> 
> Daniel Faust
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160125/ed92d7f4/attachment.html>


More information about the Plasma-devel mailing list