Review Request: Introduce Author into the Calligra family

Arjen Hiemstra djfreestyler at gmail.com
Sun Aug 19 14:08:47 BST 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106064/#review17706
-----------------------------------------------------------


Two things glare at me from this patch:

- Why put it into Words' directory instead of its own top level? The goal for this is - as I understand it - to become a separate application, so why not start off the right way? If it is a problem with Words' libraries, is it not better to fix that first than to have to risk major breakage later on?
- The prefix CAu is a really awkward prefix imo. Personally I prefer namespaces, using those you would simply declare "namespace Calligra::Author" and be done with it. But if you're sticking with prefixes, why not use CAuth? It seems to me a far more natural prefix considering CA is already taken.


words/part/author/author.rc
<http://git.reviewboard.kde.org/r/106064/#comment13865>

    Since this is a new file, version should be 1.



words/part/author/author.rc
<http://git.reviewboard.kde.org/r/106064/#comment13866>

    Most of these actions are automatically merged in. Might want to check which of them are really necessary and remove those that are not.



words/part/author/author_readonly.rc
<http://git.reviewboard.kde.org/r/106064/#comment13867>

    Does it need a read only UI? To me that does not make much sense. Also, version again.


- Arjen Hiemstra


On Aug. 19, 2012, 6:20 a.m., Gopalakrishna Bhat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106064/
> -----------------------------------------------------------
> 
> (Updated Aug. 19, 2012, 6:20 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> This is the first step in realizing [1]. Currently author resides in words/part and will eventually moved out to a directory of its own. This patch is currently a work in progress and to evaluate that we are on the right path.
> 
> Currently there is a known issue that various options of the text tool are not shown for the first time when the application is launched(see attached snapshot). Once I explicitly activate the tool by switching to a different tool and back it works fine. Any ideas what can be wrong? 
> 
> 
> [1] http://www.calligra.org/news/calligra-announces-author/
> 
> 
> Diffs
> -----
> 
>   words/part/CMakeLists.txt cdeca2b 
>   words/part/KWPart.h 5b6621b 
>   words/part/KWPart.cpp f94354a 
>   words/part/author/CAuAboutData.h PRE-CREATION 
>   words/part/author/CAuFactory.h PRE-CREATION 
>   words/part/author/CAuFactory.cpp PRE-CREATION 
>   words/part/author/CAuFactoryInit.cpp PRE-CREATION 
>   words/part/author/CAuPart.h PRE-CREATION 
>   words/part/author/CAuPart.cpp PRE-CREATION 
>   words/part/author/CAuView.h PRE-CREATION 
>   words/part/author/CAuView.cpp PRE-CREATION 
>   words/part/author/author.desktop PRE-CREATION 
>   words/part/author/author.rc PRE-CREATION 
>   words/part/author/author_readonly.rc PRE-CREATION 
>   words/part/author/authorpart.desktop PRE-CREATION 
>   words/part/author/authorrc PRE-CREATION 
>   words/part/author/main.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106064/diff/
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Text Tool
>   http://git.reviewboard.kde.org/r/106064/s/678/
> 
> 
> Thanks,
> 
> Gopalakrishna Bhat
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120819/8710daff/attachment.htm>


More information about the calligra-devel mailing list