----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105030/#review14505 -----------------------------------------------------------
Looks already quite good. Most of the things I commented is either a small fix or a style issue. stage/part/animations/strategy/KPrFormulaParser.h <http://git.reviewboard.kde.org/r/105030/#comment11466> Can you please remove the F Prefix as it is no longer needed and just don't add anything. stage/part/animations/strategy/KPrFormulaParser.h <http://git.reviewboard.kde.org/r/105030/#comment11465> Wouldn't it be better to have different functions for different number of parameters instead of needing to call it with a -999 value? stage/part/animations/strategy/KPrSmilValues.h <http://git.reviewboard.kde.org/r/105030/#comment11456> Please remove the comment. stage/part/animations/strategy/KPrSmilValues.h <http://git.reviewboard.kde.org/r/105030/#comment11457> Please remove the comment stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11464> Please move this to the initialization list of the constructor KPrSmilValues::... : m_formulaParser(0) as then it will already be initialized during compiling and not during runtime stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11458> Please fix indention and remove empty line. stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11459> Please move the else to a new line. stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11460> Please remove the comment. stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11461> this can be simplified to be return retval; stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11462> Please remove the comment. stage/part/animations/strategy/KPrSmilValues.cpp <http://git.reviewboard.kde.org/r/105030/#comment11463> This can be simplified to be QString formula = m_formulaParser->formula(); - Thorsten Zachmann On May 27, 2012, 3:04 p.m., Paul Mendez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105030/ > ----------------------------------------------------------- > > (Updated May 27, 2012, 3:04 p.m.) > > > Review request for Calligra. > > > Description > ------- > > Add support for Anim:Formula tag in Stage. (That key is part of ODF > animations specification). > > Note: animations tested don't run as smooth as in LibreOffice because > KeySplines tag is not implemented. > > > Diffs > ----- > > stage/part/CMakeLists.txt 3c7916ef7496af21e65d9a5441d5cb924829c347 > stage/part/animations/KPrAnimate.cpp > dddd1fa401d596e7e23688f950428cd0ea76b639 > stage/part/animations/strategy/KPrFormulaParser.h PRE-CREATION > stage/part/animations/strategy/KPrFormulaParser.cpp PRE-CREATION > stage/part/animations/strategy/KPrSmilValues.h > 163d78b830a151ce150192000890a395f9e273dd > stage/part/animations/strategy/KPrSmilValues.cpp > 3faafc4eb1c8783224f9f32c38106407cc219096 > stage/part/animations/strategy/KPrValueParser.h > 8f3c6ebcdf7ae9f5d938d97d518f21977b572940 > stage/part/animations/strategy/KPrValueParser.cpp > ca7fac767142e64b9711ccf835efcb9a4c242e36 > > Diff: http://git.reviewboard.kde.org/r/105030/diff/ > > > Testing > ------- > > Test some animations of documents created in Libre Office (One test document > is also uploaded) > > > Thanks, > > Paul Mendez > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
