Code review comment for lp:~vila/bzr/323111-orphan-config-option

You should add something to the user guide: aside from being needed, reading how we describe it to users will help us know if we've got it right. Starting from the text for whatsnew will be ok.

I think the always/never distinction is not the right dimension though, because it seems to assume there is one thing that we want to have either on or off. But I think what we want is, well, actually different policies, which might include:

 * conflict: mark the directory conflicted and don't remove it
 * unversion: mark the containing directory unversioned, but leave it in the tree
 * move: move them to a bzr-orphans directory in the root of the tree
 * delete: just immediately delete them

> I made the factory accept a simple callable as requiring a TreeTransform method

I think, generally, I'm against any interface taking a pure callable. (I was just regretting one I added myself in Launchpad feature flags.) So often it turns out that you also want to ask the thing passed in for a description of itself, or something like that. Passing an object with a well-known method name gives a bit more room to move.

I agree it shouldn't be bound to treetransform details.

188 + # FIXME: There is no tree config, so we use the branch one (it's weird
189 + # to define it this way as orphaning can only occur in a working tree,
190 + # but that's all we have (for now). It will find the option in
191 + # locations,conf or bazaar.conf though) -- vila 20100916
192 + conf = self._tree.branch.get_config()

Perhaps there should be a bug for that.