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