[merge] small LockableFiles cleanups

[Martin Pool]

On Wed, Nov 19, 2008 at 4:00 AM, John Arbash Meinel
<john at arbash-meinel.com> wrote:
> Martin Pool wrote:
>
> I don't quite understand the reason to rename a _foo method to __foo
> with identical functionality. I assume you are just trying to hide its
> visibility? Who was trying to call it?
>
> It seems like having the test suite actually ensure the escaping is done
> correctly is valuable, so I'm not sure why you feel like you need to
> hide it. (I presume an child class was getting it accidentally?)
>
> I'll note that there is only 1 caller of __escape that is not already a
> deprecated function, which is __init__().
>
> I'll also note that I think getting rid of escape entirely wouldn't
> actually break any functionality. At least for bzr, possibly not so for
> plugins. But I'm pretty sure that all the control file names are simple
> ascii that doesn't need escaping.
>
> In the end, I guess I just would have liked you to explain what you were
> doing, rather than having to infer it from the patch. The change seems
> okay, though it would be nice to actually be able to deprecate __escape.

There was only one external caller of it, which was the weave
repository.  I think renaming it to be totally private was on
reflection overkill.  It's already underscore-prefixed, and it's
unlikely to be used elsewhere, so just adding a note not to call it
would be enough.

I've reverted the rename, and just merged a change that cleans up the
import statements, and puts a copy of this method in weaverepo, the
only place it belongs (since it's specific to this format.)

-- 
Martin <http://launchpad.net/~mbp/>