Rick Gutleber wrote:
>
> 1. Does the new insertfrom( ) method and the InsertPolicy and SQLStream
> objects require anything new in the tests? The tests are very concise;
> I don't know if they are meant to be exhaustive.
The MySQL++ test suite's concision reflects its immaturity, not a design
goal. There should be a test for SQLStream at least in tests/* and code
to exercise insertfrom() in examples/*.
For the example, maybe something that reads a tab-delimited file of
'stock' items, makes a vector of SSQLSes from this, then inserts that
into the DB?
> 2. Do I need to make additions to the user manual to reflect these new
> pieces? Or is there someone else who handles documentation changes?
Leave userman to me.
> I attached the patch but if there is anything else I need to do, I will
> make those changes and resubmit.
Yeah, it needs some style fixes.
1. I don't see that splitting insertfrom() into public and private
halves buys much. Multiple 'success = false' lines isn't any less
redundant than multiple 'ac.rollback()' lines. (Not that I think you'll
have too many method calls on 'ac'...most of them should be at the top
and bottom of the function.) You also end up with an empty transaction
if the insert range is empty, due to starting the transaction way up
high like that, before you test the iterator range. If I'm wrong, tell
me why.
2. In the code, use tabs instead of spaces, size 4 [*], and lint your
patch for other pointless whitespace changes. This accounts for about
half your patch. As far as I can tell, 100% of the changes to
lib/query.cpp are of this type, but it happens all through the patch.
As a result, I haven't really studied the patch carefully. Too much
noise masking the signal.
3. Take the unistd.h out of examples/cpool.cpp. I fixed that problem in
svn a different way. Besides, it's unrelated to this patch. (Reading
through your patch before submitting it would have caught #2, too.)
4. Say 'svn add lib/sqlstream.* lib/insertpolicy.h' before making the
next patch. These files are missing from the one you just sent.
5. Use 0 instead of NULL. (Just style.)
6. Don't cuddle 'else'.
[*] Yes, we do use spaces instead of tabs in all of the documentation
files. I've configured my text editor to switch whitespace modes based
on the file type, so I don't often notice the change any more. We may
at some point go with spaces across the board, but for the nonce, follow
the current convention.
