Re: [Libreoffice] [libreoffice] [PUSHED] EasyHacks: Reanimating more tests in ure/sal

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.