Review Request 118735: Port libkeduvocdocument from KUrl to QUrl
Aleix Pol Gonzalez
aleixpol at kde.org
Tue Jun 17 16:31:20 UTC 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118735/#review60287
-----------------------------------------------------------
keduvocdocument/keduvocdocument.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42021>
"/Untitled"?
keduvocdocument/keduvocdocument.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42022>
Same
keduvocdocument/keduvocdocument.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42023>
Same
keduvocdocument/keduvocdocument.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42024>
Same
keduvocdocument/keduvocdocument.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42025>
Why do do an explicit copy? Doesn't
QUrl tmp(url); work?
keduvocdocument/keduvockvtml2reader.h
<https://git.reviewboard.kde.org/r/118735/#comment42026>
I wouldn't add the namespace, if all other includes in the project are not doing so.
These header namespaces have proven quite useless.
keduvocdocument/keduvockvtml2reader.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42027>
If you do ::fromLocalFile, you'll want to do toLocalFile as well. Otherwise you'll get some file:/// eventually.
Also I think you need to explicitly add the trailing slash
keduvocdocument/keduvockvtml2writer.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42029>
Maybe we want to drop these white-space changes?
keduvocdocument/keduvockvtml2writer.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42030>
+'/'+
keduvocdocument/keduvockvtml2writer.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42031>
Same
keduvocdocument/sharedkvtmlfiles.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42032>
setPath will override the fileUrl, is that what we want?
keduvocdocument/tests/converter.cpp
<https://git.reviewboard.kde.org/r/118735/#comment42033>
doesn't url already return a url? If so, no need to pass the ::fromLocalFile.
- Aleix Pol Gonzalez
On June 13, 2014, 7:26 p.m., Jeremy Whiting wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118735/
> -----------------------------------------------------------
>
> (Updated June 13, 2014, 7:26 p.m.)
>
>
> Review request for KDE Edu, Aleix Pol Gonzalez and Inge Wallin.
>
>
> Repository: libkdeedu
>
>
> Description
> -------
>
> KUrl is deprecated in frameworks, so I've ported libkeduvocdocument away from it, to QUrl.
> I also changed the top CMakeLists.txt frameworks version so it will build against frameworks from master branches.
>
>
> Diffs
> -----
>
> keduvocdocument/tests/converter.cpp 126334c
> keduvocdocument/sharedkvtmlfiles.cpp a5f45a2
> keduvocdocument/keduvoctranslation.cpp 1d7a5eb
> keduvocdocument/keduvoctranslation.h a836a77
> keduvocdocument/keduvockvtml2writer.cpp 2a043cf
> keduvocdocument/keduvockvtml2reader.h 39545f3
> keduvocdocument/keduvockvtml2reader.cpp 0d0c639
> keduvocdocument/keduvocexpression.cpp 343bc73
> keduvocdocument/keduvocdocument.cpp 878ccbf
> keduvocdocument/keduvocdocument.h a312fc1
> keduvocdocument/keduvoccontainer.h 0fc550b
> keduvocdocument/keduvoccontainer.cpp 23db0fb
> CMakeLists.txt 21696ef
> keduvocdocument/autotests/keduvocdocumentvalidatortest.cpp 781bdfa
>
> Diff: https://git.reviewboard.kde.org/r/118735/diff/
>
>
> Testing
> -------
>
> It still builds, haven't tried the tests, but I don't think the autotests cover this.
>
>
> Thanks,
>
> Jeremy Whiting
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140617/ca0fb694/attachment-0001.html>
More information about the kde-edu
mailing list