Re: [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

2014-09-22 Thread Jonathan Nieder
Michael Haggerty wrote:

> I agree with your point about overlap. I will split the documentation
> into two parts with less redundancy:
>
> * Documentation/technical/api-lockfile.txt: How to use the API.
>
> * lockfile.{c,h}: Internal implementation details.
>
> I think the implementation details would get lost among the thousand
> things in cache.h. So instead, I will add a commit on top of the patch
> series that splits out a lockfile.h header file (which I think is a good
> idea anyway), and moves the internal documentation there. Sound good?

Yep, a separate lockfile.h header file sounds sensible to me.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

2014-09-22 Thread Michael Haggerty
On 09/16/2014 11:03 PM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
> 
>> --- a/lockfile.c
>> +++ b/lockfile.c
>> @@ -4,6 +4,63 @@
>>  #include "cache.h"
>>  #include "sigchain.h"
>>
>> +/*
>> + * File write-locks as used by Git.
>> + *
>> + * When a file at $FILENAME needs to be written, it is done as
>> + * follows:
> 
> This overlaps a lot with the API doc, which makes me worry a little
> about it going out of date.  Would improving the API doc help, or if
> aspects are especially relevant here, is there a way to summarize them
> more quickly to avoid some of the redundancy?
> 
> [...]
>> + * A lock_file object can be in several states:
> 
> Would it make sense for this information to go near the definition of
> 'struct lock_file'?  That's where I'd start if I were looking for
> information on what states a lock_file can be in.

I agree with your point about overlap. I will split the documentation
into two parts with less redundancy:

* Documentation/technical/api-lockfile.txt: How to use the API.

* lockfile.{c,h}: Internal implementation details.

I think the implementation details would get lost among the thousand
things in cache.h. So instead, I will add a commit on top of the patch
series that splits out a lockfile.h header file (which I think is a good
idea anyway), and moves the internal documentation there. Sound good?

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 09/35] lockfile.c: document the various states of lock_file objects

2014-09-16 Thread Jonathan Nieder
Michael Haggerty wrote:

> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -4,6 +4,63 @@
>  #include "cache.h"
>  #include "sigchain.h"
>
> +/*
> + * File write-locks as used by Git.
> + *
> + * When a file at $FILENAME needs to be written, it is done as
> + * follows:

This overlaps a lot with the API doc, which makes me worry a little
about it going out of date.  Would improving the API doc help, or if
aspects are especially relevant here, is there a way to summarize them
more quickly to avoid some of the redundancy?

[...]
> + * A lock_file object can be in several states:

Would it make sense for this information to go near the definition of
'struct lock_file'?  That's where I'd start if I were looking for
information on what states a lock_file can be in.

My two cents,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html