Hi Maciej, First of all, thanks for submitting your patches. I've done my review of both of your patches, and I'll leave my comments below.
First, for your svl patch: 1) in SfxIntegerListItem::GetList(), you do + rList.insert( aIt, m_aList[n] ); but you can simply call push_back here to append new data to the end of the vector. Also, let's use pre-increment on iterators in the "for" loop there i.e. instead of doing aIt++, do ++aIt. Post-increment on iterators is generally more expensive than pre-increment, so it's best to stick with pre-increments. 2) Your new constructors take arrays of different integer type; one takes std::vector<sal_uLong> while the other takes uno::Sequence<sal_Int32>. Let's just stick with one type and use sal_Int32 in both cases since that's the integer type that this class uses internally. And for your sc patch: 3) In ScTabViewShell::Execute() you do - SvULongs aIndexList( 4, 4 ); + ::std::vector < sal_uLong > aIndexList( 4, 4 ); but this actually changes the behavior of the code. The arguments to SvULong's constructor controls how the internal array is laid out during initialization but it doesn't populate the container with data. Vector's ctor arguments OTOH populate the container with data. So, your new code initializes aIndexList with 4 elements of value 4, which is not what the code intends to do. You can simply do without passing any arguments to aIndexList there. 4) Last but not least, please state that you are submitting your patches under LGPLv3+/MPL 1.1. That's all I can think of. Let's get these points revised, and I'll push the changes to master. Best, Kohei -- Kohei Yoshida, LibreOffice hacker, Calc <kyosh...@novell.com> _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
