[Monotone-devel] comments on net.venge.monotone.workspace-merge.migration

[Nathaniel Smith]

Two substantive comments:

Regarding
  
http://thread.gmane.org/gmane.comp.version-control.monotone.devel/7752/focus=7756
Do you have any opinion?  It also occurs to me since I wrote that that
there actually might be utility for us in keeping _MTN/revision as it
is now -- because the UI for workspace merge (and friends) probably
_will_ want to break symmetry, e.g., to have THIS and OTHER selectors
(where which parent each refers to depends on what revision you were
on before you ran 'merge').


Regarding tests/migrate_workspace:
  I can't really tell how I would add a new test to this, or if it's
  even testing migration from format 2.  (I see that it tests that
  running the migration command with a workspace generated by whatever
  monotone is being tested, but we should assume that eventually that
  will stop being the same as "format 2".)

  Compare tests/schema_migration, where 1) it's stupid-simple to drop
  in a new test after you change the format, 2) you are supposed to do
  this for your new format when you change the format, so that the
  person adding the test is the same person who already has swapped in
  the details of the format that they're adding a test for.


Some more trivial things:

+// This is the oldest released version of monotone that supports the current
+// format.
+static const char first_version_supporting_current_format[] = "0.29";

^^ needs to be bumped, obviously :-).


+bool
+roster_t::get_attr(split_path const & pth,
+                   attr_key const & name,
+                   attr_value & val) const
+{
+  if (has_node(pth))
+    {

Not I(has_node(pth))?  Would someone ever use this with a non-existent
pth?


+-- Delete a directory tree constructed as above.
+function deltree(tree)

Does this have any purpose in particular?  (It seems to be unused, and
rm -rf would do just as well, and we'd generally like to leave files
around anyway for forensics when a test fails.)


comment at the top of work.hh would be good to update (this is the one
that says, for instance:
// _MTN/revision     -- contains the id of the checked out revision
// _MTN/work         -- (optional) a set of added, deleted or moved pathnames
//                      this file is, syntactically, a cset
)

-- Nathaniel

-- 
  /* Tell the world that we're going to be the grim
   * reaper of innocent orphaned children.
   */
-- Linux kernel 2.4.5, main.c