On Mon, May 17, 2010 at 2:33 AM, Thomas Keller
<address@hidden> wrote:
\2) I'd like to get my nvm.experiment.database-management branch ready
and merged in as well, so the change I did earlier to mtn setup (which
now creates a database if none is given) is changed to "create a
database in the default location or use the default database there". The
code compiles already and partially works, but I had to do a couple of
non-related changes to make database objects easier constructable and
other things, which make mtn currently crash badly. I'll commit my
current state tonight, so others have time to look at it. Maybe I'm on a
wrong path there.
I had a quick look over your changes and they seem fairly reasonable. A few minor things:
We seem to be using lots of redundant std:: prefixes in various .cc files (not just in your changes) where we also have using std::foo declarations to avoid needing the std:: prefixes throughout the implementation. Do people have a general preference for explicit prefixes? Personally I much prefer a using declaration and less cluttered sources, iterators are already bad enough, sprinkling them with std:: makes them worse! ;)
Some of your changelog comments use very long lines and are somewhat hard to read without a tool that does automatic, but careful, wrapping (viewing these in emacs diff-mode is not particularly pleasant). Can you try and wrap these at some reasonable length please?
Based on all the shenanigans with ".mtn" extensions it seems like maybe the path handling code could use a more formal concept of extension.
A shared_ptr for the lazy_rng would probably be better than a raw pointer.
We should probably have a ":memory:" constant somewhere, rather than multiple string instances.
I don't think you want to set the foo_given flags when reading options from the options file do you? Isn't the point of these to know whether the user specified such an option on the command line?
Cheers,
Derek