[Libreoffice] [libreoffice] [PUSHED] EasyHacks: Reanimating more tests in ure/sal
David Tardon
dtardon at redhat.com
Tue Feb 8 00:16:54 PST 2011
On Sun, Feb 06, 2011 at 11:10:07PM +0100, Wilhelm Pflüger wrote:
> More reanimated tests. The osl/file part is not satisfying. I had to
> switch off some tests in osl_old_test_file.cxx. Maybe it is not worth to
> keep it.
>
> These patches are LGPLv3+/MPL.
>
Nice work! I have a couple of comments, nevertheless:
1. Try to avoid changing whitespace in unrelated code, as it makes it
harder to spot the actual changes. If you really feel that you must
do it, use a separate commit .-) I removed a lot of hunks that
contained only a whitespace changes this time, but left it in other
hunks.
2. When re-enabling old code (like in this case), avoid adding new
functionality (that is not absolutely needed for the code to work).
Again, it makes it harder for the reviewer. It also complicates
reverting a change in a test, should it be found that it does not
work as expected. Of course, your changes in the osl_Directory tests
are good and I encourage you sincerely to continue in enhancing the
tests; just commit the enhancements separately, please :-)
3. (a nitpick) bTest == sal_False is customarily written as !bTest .-)
4. (I forgot to note this the last time) Special preprocessor magic is
required for the case of building with internal STLPort, but external
CppUnit :-) See commit 5c5e8b76f27f45a660455c236496513201afd911 in
ure.
D.
More information about the LibreOffice
mailing list