----------------------------------------------------------- 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
