Review Request 116067: Implement WebSocket type.

[Anton Kreuzkamp]

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


One issue and some minor nags. But nice code overall! Thank you! :)


src/qtcore.js
<https://git.reviewboard.kde.org/r/116067/#comment35780>

    WebSocket.Null was not defined above.



src/qtcore.js
<https://git.reviewboard.kde.org/r/116067/#comment35781>

    No need of using self here. Just use this.
    Self is only needed if we have e.g. DOM callbacks, where this is pointing to the DOM element instead of the QML object.



src/qtcore.js
<https://git.reviewboard.kde.org/r/116067/#comment35782>

    Two things here:
    1. QmlWeb uses camelCase instead of snake_case. Please use camelCase as well, for better consistency.
    2. You can just use
    function connectSocket() {
    }
    here, I think it's better readable and might bring some small memory optimizations on some browsers.



src/qtcore.js
<https://git.reviewboard.kde.org/r/116067/#comment35784>

    Same as above (camelCase + function).



src/qtcore.js
<https://git.reviewboard.kde.org/r/116067/#comment35786>

    Just a small one, but I tend to omit the braces for oneliners. Just for consistency, please do that as well. :)


- Anton Kreuzkamp


On Feb. 25, 2014, 11:45 p.m., Nikita Skovoroda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116067/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 11:45 p.m.)
> 
> 
> Review request for QML Web and Anton Kreuzkamp.
> 
> 
> Repository: qmlweb
> 
> 
> Description
> -------
> 
> Implement WebSocket type.
> 
> A simple wrapper around javascript WebSocket class ( http://caniuse.com/websockets ).
> 
> API compatible with Qt.WebSockets (see https://qt.gitorious.org/qt/websockets ).
> The Qt.WebSockets module will be included in Qt 5.3 release: http://qt-project.org/wiki/New-Features-in-Qt-5.3#a12a092a739cea3499350adb459d86b0 .
> 
> 
> Diffs
> -----
> 
>   src/qtcore.js c219e2e 
> 
> Diff: https://git.reviewboard.kde.org/r/116067/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> 
> I could send and recieve messages and get status updates.
> 
> 
> Thanks,
> 
> Nikita Skovoroda
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/qmlweb/attachments/20140226/956e5329/attachment.html>