[Merge] lp:~dandrader/qtubuntu/betterSessionName into lp:qtubuntu

Daniel d'Andrada daniel.dandrada at canonical.com
Wed Jun 1 15:21:27 UTC 2016


On 01/06/2016 12:01, Gerry Boland wrote:
> Review: Needs Fixing
>
> +for (int i = 0; i < args.count(); ++i) {
> +   QString arg = args[i];
> We have Q_FOREACH

I personally find this macro ugly, but if you fancy it...
But it is less code and thus less error prone. Done.

> Do you really need to read the FileInfo object? Why not just drop the ".qml" from the file name?
Yes. The argument could be a full file path like this:

/foo/bar/whatever/zumba.qml

Which would make for a cumbersome session name. QFileInfo neatly and 
safely parses that filepath for me.

I like the ".qml" suffix as it tells us it's a plain qml app run via 
qmlscene.

> + u_application_id_new_from_stringn(sessionName.data(), sessionName.count());
> Does the api expect sessionName to always exist, or will it make a copy itself?

It makes a copy. What a horrible API it would be otherwise.

-- 
https://code.launchpad.net/~dandrader/qtubuntu/betterSessionName/+merge/296198
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu.



More information about the Ubuntu-reviews mailing list