[Development] [API Review Request] QWebsockets

[Shane Kearns]

There's a fair amount of legacy code using other methods for implementing
the private class pointer. Q_Q and Q_D macros are preferred though.

You don't need to merge your .cpp files, we generally have one .cpp per
class (though trivial classes don't always have their own file).
The point is that they don't need to be named xxx_p.cpp, you can just use
xxx.cpp

The _p.h suffix has a special meaning to Qt's toolchain - the header file
won't be installed to the include path.

There are two options to run autobahn on the test infrastructure:
If it's just a python script, we have python on the test machines for
compiling webkit.
You could launch python as a child process at the start of your test, and
kill it at the end (in your cleanup function).

If that doesn't work, the other option would be to install it in the test
server.
Our test server runs a number of proxies and servers which we test Qt
against.
This is a linux virtual machine configured with puppet scripts (which are
in the qtqa git repository)
For this option you'd need to setup a test server locally - there are
details in this blog post:
http://blog.qt.digia.com/blog/2011/06/27/qt-test-server-setup-for-network-auto-tests/

It's fairly easy if you're familiar with using vmware or virtualbox


On 25 August 2013 21:48, Kurt Pattyn <pattyn.kurt at gmail.com> wrote:

> Ok, tabs has been converted into 4 spaces.
>
> Regarding the *_p.cpp files: I just did this to keep the file sizes at a
> reasonable size; if it is a style requirement, I can merge those files.
> The same holds for the other files (DataProcessor, …). If I merge them
> with QWebSocket.cpp, then that file will be really big; isn't that a
> problem then?
>
> For the Q_D and Q_Q macros, I will look that up (unless someone has a
> reference to the rule); I found Qt code that was just using d_ptr and
> q_ptr, or even d_func().
> What's the general rule?
>
> Yeah, unit tests are incomplete; I am aware of that. Currently, the only
> tests I have, is against the Autobahn Testsuite (both client and server),
> which I start manually. That suite is testing all normal, border and error
> conditions (based on RFC 6455). Memory leaks have been tested with
> Instruments on Mac OSX. For the rest, the code is making use of QTcpSocket
> and QTcpServer. I'll expand the unit tests in the coming days. I suppose I
> cannot run Autobahn on the Qt infrastructure?
>
> Best regards,
>
> Kurt
>
> On 25 Aug 2013, at 22:29, Matt Broadstone <mbroadst at gmail.com> wrote:
>
> On Sun, Aug 25, 2013 at 3:49 PM, Kurt Pattyn <pattyn.kurt at gmail.com>wrote:
>
>> Hi,
>>
>> I am about to move the QWebSockets project (see:
>> http://github.com/KurtPattyn/QWebSockets) to the playground.
>> Could you please review the API?
>>
>>
> Just some quick stylistic observations:
>
> * you don't usually make a class_p.cpp (implementation) file, you only
> need to do that for the header so that you don't export those symbols
>
> * looks like you are using tabs all over the place
>
> * I think it's part of the style guide to use the Q_D and Q_Q macros to
> access your classes d and q pointers, respectively.
>
> * imho a lot of this code could be squashed together so as to reduce the
> number of files:
>    - qwebsocketprotocol can just go into the "global" header, since it
> just defines some enums
>    - classes like "DataProcessor" and "HandshakeResponse" could probably
> just move to the implementation of QWebSocket
>
> * unit tests are woefully incomplete, also you seem to be running qtestlib
> manually.. I would suggest taking a look at Qts test dir or something like
> it for inspiration here (many folders with a set of tests in them). This
> part is probably going to be most difficult, as you'll be writing tests
> after you've seemingly completed the implementation.
>
> Cheers,
> Matt
>
> Notes:
>>
>>    1. The public API is QWebSocket and QWebSocketServer; all other
>>    classes are internal.
>>    The APIs of both classes were modelled after QAbstractSocket and
>>    QTcpServer. This should it make easier to use the API.
>>    2. I have also tried to make the directory structure compliant with
>>    the Qt guidelines; so, if there are any mistakes, it would be good to know
>>    as well
>>    3. I created a qdocconfig file, but it misses links to the Qt library
>>    classes
>>    4. The sync.profile has been created by looking at the QtSerialPort
>>    project, but I am not sure if that is correct as well
>>    5. Unittests still need work to be up to the Qt standards
>>
>>
>> Thanks for your time.
>>
>> _______________________________________________
>> Development mailing list
>> Development at qt-project.org
>> http://lists.qt-project.org/mailman/listinfo/development
>>
>>
>
>
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20130827/182e14f7/attachment.html>