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. _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice