Fredag 17. januar 2014 17.56.50 skrev Kurt Pattyn: > On 17 Jan 2014, at 14:10, Gladhorn Frederik <[email protected]> wrote: > > Hi Kurt, > > > > I'm quite impressed with the state of the websockets module. > > Thanks! > > > I tried reviewing the code, but I have to admit that I just started > > reading the RFC a bit, so here are some first random notes. > > > > When websockets becomes part of a Qt module you can consider using > > QObjectPrivate which makes the d pointer implementation slightly cleaner > > and removes the need for duplicate d pointers (one from qobject one for > > the class's own implementation. > Okay, will fix that. > > > In the public interface I'd generally declare signals with const refs, > > alone for consistency. > Done. Your patch has been reviewed and merged (thanks for that). > > > While not really harmful there is generally no point in explicitly > > initializing empty strings, I'd personally remove the QString() calls > > from the class initialzers just to remove a few lines. > > > > qsslserver newEncryptedConnection is never emitted, I didn't look into > > what it means though. > Hmmm, I’ll have a look at that one. > > > qwebsockethandshakeresponse: m_acceptedExtension doesn't seem used, > > instead there is a string list passed through, I suppose it can simply be > > removed, it also has a getter that will always return an empty string. > Will check this also. > > > I was reading the RFC a bit, in section 9.1 there are some notes about the > > extensions: > > > > const QList<QString> matchingExtensions = > > > > supportedExtensions.toSet().intersect(request.extensio > > ns().toSet()).toList();> > > does no longer guarantee the order of extentions - according to sec 9.1. > > this is important. > Right. Will fix this. > > > The same seems to go for matchingProtocols - the first one is picked, > > since QSet doesn't guarantee any order it is basically chosen by random. > > I don't know how significant that is but I assume the client's first > > choice (?) should be taken. > Hmm, the list is sorted in descending order, and the server picks the latest > protocol version that it supports. That seems fine to me, but I’ll > double-check the RFC.
My bad, you are right, I overlooked the sorting. Greetings, Frederik > > > Cheers, > > Kurt > > > Greetings, > > Frederik > > > > From: [email protected] > > [[email protected]] on > > behalf of Kurt Pattyn [[email protected]] Sent: Thursday, January 16, > > 2014 12:51 PM > > To: [email protected] > > Cc: Heikkinen Jani; '[email protected]' > > ([email protected]); [email protected]; > > [email protected] Subject: Re: [Development] Qt 5.3 Feature freeze > > is coming quite soon... > > > > I would like to propose the QtWebSockets module as a new feature for Qt > > 5.3 (see https://qt.gitorious.org/qtplayground/websockets/source/master) > > > > There are a number of requests in Jira: > > https://bugreports.qt-project.org/issues/?jql=labels%20%3D%20websockets > > asking to include web socket functionality in Qt, so maybe it is a good > > time to include it now. Besides that, EnginIo could make use of this > > module as well (currently it uses an own implementation of web sockets). > > > > QtWebSockets is fully RFC6455 compliant (see > > http://tools.ietf.org/html/rfc6455) and is successfully tested against > > the latest Autobahn TestSuite. Performance is OK as well (test report is > > not included because of too big). > > > > There is a C++ interface as well as a QML module. > > The C++ API consists of the following classes: > > QWebSocket > > QWebSocketServer > > QWebSocketProtocol > > > > The QML API consists of the following component: > > WebSocket (included in the Qt.WebSockets 1.0 module). > > > > I see 2 options: either add the functionality to the QtNetwork module, or > > add it as a Qt add-on. Maybe adding it to QtNetwork would be a ‘natural’ > > place: QWebSocket would then live besides QTcpSocket, QWebSocketServer > > would live besides QTcpServer. > > > > The module compiles on all reference platforms, has automated unit tests > > and manual unit tests (against Autobahn TestSuite), is fully documented, > > and has a number of examples. The module is included in the CI system of > > Qt (see > > http://testresults.qt-project.org/ci/WebSockets_master_Integration/). > > > > > > Please raise your votes. > > > > Cheers, > > > > Kurt > > > > On 16 Jan 2014, at 09:28, Heikkinen Jani <[email protected]> wrote: > >> Hi all, > >> > >> I want to remind you all that Qt 5.3 feature freeze is coming pretty > >> soon. Feature freeze for Qt 5.3 is 14th Feb 2014 so there is only 4 > >> weeks left for implementing new features. > >> > >> Qt 5.3 schedule can be found here: > >> http://qt-project.org/wiki/Qt-5.3-release . There is also link to the Qt > >> 5.3 new features page > >> (http://qt-project.org/wiki/New-Features-in-Qt-5.3). You can start > >> collecting list of new features there… > >> > >> Note: Let’s keep the feature freeze date! If your feature isn’t ready at > >> that point let’s then move it to Qt 5.4 release instead of taking in it > >> in Qt 5.3 and fighting with it whole release time… This is also action > >> point for each maintainer: Make sure your component is ready for feature > >> freeze at that date. > >> > >> Just a reminder: > >> In the feature freeze all new functionality must > >> > >> - Compile on all reference platforms (If a module/feature is only for one > >> platform, make sure qmake/make does nothing on the other platforms) - > >> Have tests. Automated tests should cover as much as possible of the new > >> functionality. If certain areas are not covered by automated tests, > >> there must be clarification how testing will be done for those - Have > >> documentation. No undocumented public API. Basic docs have to be there, > >> only polishing should still be required after the freeze - Have > >> examples. Have some examples showing how to use the API. Examples need > >> to be linked to from documentation. > >> > >> In addition, new modules need to > >> > >> - Follow the branching scheme. dev/stable/release should be there. A new > >> module can be ok to only have dev, with stable being created at > >> branching time. - Have a CI system. New modules that are going to be > >> part of Qt releases need to have a CI system set up > >> > >> Br, > >> Jani > >> _______________________________________________ > >> Development mailing list > >> [email protected] > >> http://lists.qt-project.org/mailman/listinfo/development -- Best regards, Frederik Gladhorn Senior Software Engineer - Digia, Qt Visit us on: http://qt.digia.com _______________________________________________ Development mailing list [email protected] http://lists.qt-project.org/mailman/listinfo/development
