On Wed, Jun 11, 2008 at 04:12:23PM -0500, Nicolas Williams wrote: > On Wed, Jun 11, 2008 at 08:38:48PM +0100, Alan Maguire wrote: > > No need as far as I can see - testing indicates > > these all just work, since scf_entry_add_value() and > > the backend do the right thing. I think what we?d need to do > > is ensure that manpages document the ordering > > behaviour. > > Perhaps I misunderstood. I thought you were adding a notion of ordered > vs. unordered. But perhaps you're merely imposing order always on all > props added from the time this feature is available. > > The former implies UI and API changes (though not necessarily > backwards-incompatible) by which users/programs could specify whether > they want a property's values to be ordered. > > The latter implies no UI changes nor API changes.
Reading the webrev helps! :) :) BTW, check out: - $SRC/cmd/idmap/idmapd/schema.h - $SRC/cmd/idmap/idmapd/dbutils.c:init_dbs() - $SRC/cmd/idmap/idmapd/dbutils.c:init_db_instance() idmapd does schema upgrades too. Your approach of defaulting value_order to zero is clever. I like it. Comments: - Why not update BACKEND_SCHEMA_VERSION? One possible answer: because minor schema changes don't warrant the complexity of updating schema_version. It's easy enough to detect the actual schema version by testing whether some statement compiles or by looking at sqlite_master (idmapd does the latter, your changes do the former). - Given that the schema upgrade happens when the db is opened, then why bother with backend_is_upgraded()? Oh, I see, because if you can open the thing but can't upgrade then you treat the backend as read-only. I think you can address both comments by adding some comments to the source :) Nico --
