From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 07 Jul 2013 00:11:48 GMT
Subject: Re: [PATCH 1/2] Git.pm: add new temp_is_locked function
Message-ID: <20130707001148.GD30132@google.com>
In-Reply-To: <D34E9C76-89B8-44E6-B364-C71B2FC2AC52@gmail.com>

Hi,

Kyle McKay wrote:

> The temp_is_locked function can be used to determine whether
> or not a given name previously passed to temp_acquire is
> currently locked.
[...]
> +=item temp_is_locked ( NAME )
> +
> +Returns true if the file mapped to C<NAME> is currently locked.
> +
> +If true is returned, an attempt to C<temp_acquire()> the same
> C<NAME> will
> +throw an error.

Looks like this was line-wrapped somewhere in transit.

More importantly, it's not obvious from the above description what
this function will do.  What is a typical value of NAME?  Is it a
filename, an arbitrary string, a reference, or something else?  Is
this a way of checking if a file is flocked?  What is a typical way to
use this function?

Looking more closely, it looks like this is factoring out the idiom
for checking if a name is already in use from the _temp_cache
function.  Would it make sense for _temp_cache to call this helper?

When is a caller expected to call this function?  What guarantees can
the caller rely on?  After a call to temp_acquire(NAME), will
temp_is_locked(NAME) always return true until temp_release(NAME) is
called?  Does this only work within the context of a single process or
can locks persist beyond a process lifetime?  Do locks ever need to be
broken?

I didn't spend a lot of time trying to find the answers to these
questions because I want to make sure that people using Git.pm in the
future similarly do not have to spend a lot of time.  So hopefully a
documentation change could fix this.

Thanks and hope that helps,
Jonathan