[PATCH 2 of 4] lock message: calculate lock description file name

[Pierre-Yves David]

On 03/08/2016 08:50 PM, Tuli Uchitel wrote:
> # HG changeset patch
> # User Tuli Uchitel <tuli at fb.com>
> # Date 1457463708 0
> #      Tue Mar 08 19:01:48 2016 +0000
> # Branch stable
> # Node ID 1ffef024963c56879289caf534575d0c72f4b7e9
> # Parent  9fb33594d0e791034812adb34e12a4a3ee636418
> lock message: calculate lock description file name
>
> implement a method that creates a name for a file that will hold description about the lock that has been acquired

Why do we need complicated logic for that? is there any special magic 
involved of are we just handling the .hg/lock vs .hg/store/lock case?


>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py

You get extra suspicious for adding method on localrepository. Can we 
stick this somewhere else?

> @@ -1237,6 +1237,13 @@
>                   continue
>               ce.refresh()
>
> +    def _getlockdescriptionfilename(self, lockfilepath):

Can we get a docstring here?

> +        lockdescriptionfilename = None
> +        if isinstance(lockfilepath, basestring):

‽ Why do we need this isinstance check here. Usage of "isinstance" is 
usually a code smell in Python. What are we protecting ourself against 
here? If we have a real reason (I would be sad) we should at least 
document it.

> +            lockfilename = lockfilepath.split("/")[-1]

Please use os.path.split for this purpose


> +            lockdescriptionfilename = "%s.description" % lockfilename
> +        return lockdescriptionfilename
> +
>       def _lock(self, vfs, lockname, wait, releasefn, acquirefn, desc,
>                 inheritchecker=None, parentenvvar=None):
>           parentlock = None

-- 
Pierre-Yves David