Farid Zaripov wrote:
-----Original Message-----
From: Farid Zaripov [mailto:[EMAIL PROTECTED]
Sent: Friday, December 22, 2006 4:05 PM
To: stdcxx-dev@incubator.apache.org
Subject: [PATCH] std::list container tests
The patch located here: http://zaripov.kiev.ua/src/tests.patch
The changelog will be posted later.
The changelog:
Thanks. I've spent some time going through the patch and while
I still like your changes I'm not completely comfortable with
their extent or with lumping them all together. For instance
the renaming of class X and separating it from <alg_test.h>
into a header of its own, while a good change, should be done
in a separate patch, ideally with a transitional typedef for
X to make it possible to commit the patch without touching
all the tests that use X. Another patch should then change
the tests to use the new name(s) and remove the transitional
typedef.
So for starters, I would like to see the patch broken up into
as many smaller ones as reasonable. Changes that are logically
independent of all others (e.g., the renaming of symbols)
should be made separately, ideally with some transitional
solution in place (as I described above).
Changes to the iterator helper templates (ConstFwdIter etc.)
should also be made separately.
If you are moving things from one header or source to another
it's good to do the move without making any other changes so
as to make it possible to review each types of changes
separately (otherwise there's no easy way to tell that in
addition to moving a class to a new header you also changed
some of its members).
Also, it will be better and cleaner (and easier to review) to
first make all the test driver changes and add the new tests
only when the driver is done.
Other than these general issues, please make sure to use the
appropriate macros (e.g., _RWSTD_SIZE_T, or _TRY and _CATCH)
rather than the actual C++ constructs to make the code easily
portable to older platforms.
Also, I notice you changed the comment describing the
is_sorted_lt template from
// returns true iff a sequence of values
to
// returns true if a sequence of values
The "iff" is not a typo, it's a shorthand for "IF and only iF."
Thanks
Martin
