Review Request 119467: Implement the main UI in QML (Kadixt patches 1/3)

David Edmundson david at davidedmundson.co.uk
Fri Jul 25 15:21:53 UTC 2014


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


First round of comments.


src/CMakeLists.txt
<https://git.reviewboard.kde.org/r/119467/#comment43916>

    Remove this.



src/CMakeLists.txt
<https://git.reviewboard.kde.org/r/119467/#comment43917>

    There's an install directory command.



src/call-window.cpp
<https://git.reviewboard.kde.org/r/119467/#comment43918>

    we don't want these TODOs



src/call-window.cpp
<https://git.reviewboard.kde.org/r/119467/#comment43919>

    All this needs cleaning.



src/dtmf-handler.h
<https://git.reviewboard.kde.org/r/119467/#comment43920>

    random TODO



src/dtmf-qml.cpp
<https://git.reviewboard.kde.org/r/119467/#comment43921>

    remove



src/dtmf-qml.cpp
<https://git.reviewboard.kde.org/r/119467/#comment43923>

    const &
    
    digit isn't a good variable name, might not be a digit.



src/dtmf-qml.cpp
<https://git.reviewboard.kde.org/r/119467/#comment43924>

    why not just set this to Tp::DTMFEvent in the first place?



src/dtmf-qml.cpp
<https://git.reviewboard.kde.org/r/119467/#comment43922>

    '' rather than ""



src/qml/Main.qml
<https://git.reviewboard.kde.org/r/119467/#comment43925>

    All need trimming.



src/qml/Main.qml
<https://git.reviewboard.kde.org/r/119467/#comment43926>

    spelling.



src/qml/Main.qml
<https://git.reviewboard.kde.org/r/119467/#comment43927>

    These aren't very declarative.
    
    Suggest we expose as root context properties rather than functions on the QML.



src/qml/Main.qml
<https://git.reviewboard.kde.org/r/119467/#comment43928>

    i18n


- David Edmundson


On July 25, 2014, 3:08 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119467/
> -----------------------------------------------------------
> 
> (Updated July 25, 2014, 3:08 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-call-ui
> 
> 
> Description
> -------
> 
> Implement the main UI in QML (Kadixt patches 1/3)
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 8c39e7c 
>   src/CMakeLists.txt 250aeb5 
>   src/call-window.h 07fd01e 
>   src/call-window.cpp c38112d 
>   src/call-window.ui 32c7dad 
>   src/dtmf-handler.cpp d8d7970 
>   src/dtmf-qml.h PRE-CREATION 
>   src/dtmf-qml.cpp PRE-CREATION 
>   src/dtmf-widget.h 9e1bc73 
>   src/dtmf-widget.cpp f3436b2 
>   src/dtmf-widget.ui 67f60b9 
>   src/qml-interface.h PRE-CREATION 
>   src/qml-interface.cpp PRE-CREATION 
>   src/qml/Main.qml PRE-CREATION 
>   src/qml/core/Button.qml PRE-CREATION 
>   src/qml/core/Dtmf.qml PRE-CREATION 
>   src/qml/core/DtmfButton.qml PRE-CREATION 
>   src/qml/core/Label.qml PRE-CREATION 
>   src/qml/core/Separator.qml PRE-CREATION 
>   src/qml/core/ToggleButton.qml PRE-CREATION 
>   src/qml/core/Toolbar.qml PRE-CREATION 
>   src/qml/core/Tooltip.qml PRE-CREATION 
>   src/callwindowui.rc 6c390b9 
>   src/dtmf-handler.h 91960dc 
> 
> Diff: https://git.reviewboard.kde.org/r/119467/diff/
> 
> 
> Testing
> -------
> 
> Rebased, patched and cherry-picked until I was red in the face.
> 
> Compiles, not tested beyond that.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140725/9ced5e86/attachment-0001.html>


More information about the KDE-Telepathy mailing list