Review Request 122877: Move Parley's editor model classes into libkeduvocdocument

Inge Wallin inge at lysator.liu.se
Mon Mar 9 20:59:18 UTC 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122877/#review77230
-----------------------------------------------------------



keduvocdocument/keduvoccontainermimedata.h
<https://git.reviewboard.kde.org/r/122877/#comment53036>

    I am not sure what the rest of the library says, but I have the feeling that it's licensed under the LGPL. We may have to ask frederik gladhorn if he is willing to relicense these files under the LGPL also. And also check with the other authors.  I know that I have no problem with it.



keduvocdocument/keduvoccontainermodel.cpp
<https://git.reviewboard.kde.org/r/122877/#comment53037>

    The includes should all be using the same style.  If the other includes from the library use <> instead of "" then these should also do that.



keduvocdocument/keduvocreadonlycontainermodel.cpp
<https://git.reviewboard.kde.org/r/122877/#comment53038>

    You should add your own copyright too, at least to the files where you made real changes.



keduvocdocument/keduvocvocabularymodel.cpp
<https://git.reviewboard.kde.org/r/122877/#comment53039>

    strange indentation



keduvocdocument/keduvocvocabularymodel.cpp
<https://git.reviewboard.kde.org/r/122877/#comment53040>

    A tooltip sounds like something that belongs in an application rather than in a library.  This should probably be in Parley instead of here.  Check if it's possible to move or if it really needs to be here.
    
    Also check if there are more tooltips in other models.



keduvocdocument/keduvocvocabularymodel.cpp
<https://git.reviewboard.kde.org/r/122877/#comment53041>

    "Word Class"



keduvocdocument/keduvocwordclassmodel.h
<https://git.reviewboard.kde.org/r/122877/#comment53042>

    I think we should either comment out these functions or actually implement them. It's ok to have TODO's like this in an application but not in a library.


The code looks very clean, which is not strange since this is only moved from a long working implementation, not a new implementation. But there are still a few simple issues and one serious one: namely the GPL vs LGPL license issue. See comment inside.

- Inge Wallin


On March 9, 2015, 9:05 p.m., Rahul Chowdhury wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122877/
> -----------------------------------------------------------
> 
> (Updated March 9, 2015, 9:05 p.m.)
> 
> 
> Review request for KDE Edu, Inge Wallin and Jeremy Whiting.
> 
> 
> Repository: libkeduvocdocument
> 
> 
> Description
> -------
> 
> The model classes of parley's editor is moved into libkeduvocdocument. As a result we can have an editor in the library itself, and the applications won't need to keep their separate editors.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 09aed2b 
>   keduvocdocument/CMakeLists.txt 6eeba12 
>   keduvocdocument/keduvoccontainermimedata.h PRE-CREATION 
>   keduvocdocument/keduvoccontainermimedata.cpp PRE-CREATION 
>   keduvocdocument/keduvoccontainermodel.h PRE-CREATION 
>   keduvocdocument/keduvoccontainermodel.cpp PRE-CREATION 
>   keduvocdocument/keduvoclessonmodel.h PRE-CREATION 
>   keduvocdocument/keduvoclessonmodel.cpp PRE-CREATION 
>   keduvocdocument/keduvocreadonlycontainermodel.h PRE-CREATION 
>   keduvocdocument/keduvocreadonlycontainermodel.cpp PRE-CREATION 
>   keduvocdocument/keduvocvocabularymimedata.h PRE-CREATION 
>   keduvocdocument/keduvocvocabularymimedata.cpp PRE-CREATION 
>   keduvocdocument/keduvocvocabularymodel.h PRE-CREATION 
>   keduvocdocument/keduvocvocabularymodel.cpp PRE-CREATION 
>   keduvocdocument/keduvocwordclassmodel.h PRE-CREATION 
>   keduvocdocument/keduvocwordclassmodel.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/122877/diff/
> 
> 
> Testing
> -------
> 
> It builds and runs with no errors.
> 
> 
> Thanks,
> 
> Rahul Chowdhury
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20150309/fabe578e/attachment-0001.html>


More information about the kde-edu mailing list